From e45939d18f5e4d8bda5232a44c4e25d090344ae3 Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Sun, 10 Apr 2016 23:48:49 -0700 Subject: [PATCH] Make sure ResourceCacheSharedItems deals with strong refs --- .../GlobalServicesScriptingInterface.cpp | 4 +- interface/src/ui/Stats.cpp | 11 ++--- libraries/networking/src/ResourceCache.cpp | 42 ++++++++++++++----- libraries/networking/src/ResourceCache.h | 12 +++--- 4 files changed, 42 insertions(+), 27 deletions(-) diff --git a/interface/src/scripting/GlobalServicesScriptingInterface.cpp b/interface/src/scripting/GlobalServicesScriptingInterface.cpp index d56a987d2b..e6d87a0260 100644 --- a/interface/src/scripting/GlobalServicesScriptingInterface.cpp +++ b/interface/src/scripting/GlobalServicesScriptingInterface.cpp @@ -122,8 +122,8 @@ void DownloadInfoResultFromScriptValue(const QScriptValue& object, DownloadInfoR DownloadInfoResult GlobalServicesScriptingInterface::getDownloadInfo() { DownloadInfoResult result; - foreach(QSharedPointer resource, ResourceCache::getLoadingRequests()) { - result.downloading.append(resource ? (resource->getProgress() * 100.0f) : 0.0f); + foreach(auto resource, ResourceCache::getLoadingRequests()) { + result.downloading.append(resource->getProgress() * 100.0f); } result.pending = ResourceCache::getPendingRequestCount(); return result; diff --git a/interface/src/ui/Stats.cpp b/interface/src/ui/Stats.cpp index d19f7e2cab..d1cb4cf3c2 100644 --- a/interface/src/ui/Stats.cpp +++ b/interface/src/ui/Stats.cpp @@ -196,7 +196,7 @@ void Stats::updateStats(bool force) { STAT_UPDATE(audioMixerPps, -1); } - QList> loadingRequests = ResourceCache::getLoadingRequests(); + auto loadingRequests = ResourceCache::getLoadingRequests(); STAT_UPDATE(downloads, loadingRequests.size()); STAT_UPDATE(downloadLimit, ResourceCache::getRequestLimit()) STAT_UPDATE(downloadsPending, ResourceCache::getPendingRequestCount()); @@ -205,8 +205,7 @@ void Stats::updateStats(bool force) { bool shouldUpdateUrls = _downloads != _downloadUrls.size(); if (!shouldUpdateUrls) { for (int i = 0; i < _downloads; i++) { - auto request = loadingRequests[i].lock(); - if (!request || request->getURL().toString() != _downloadUrls[i]) { + if (loadingRequests[i]->getURL().toString() != _downloadUrls[i]) { shouldUpdateUrls = true; break; } @@ -215,10 +214,8 @@ void Stats::updateStats(bool force) { // If the urls have changed, update the list if (shouldUpdateUrls) { _downloadUrls.clear(); - foreach (QSharedPointer request, loadingRequests) { - if (request) { - _downloadUrls << request->getURL().toString(); - } + foreach (auto resource, loadingRequests) { + _downloadUrls << resource->getURL().toString(); } emit downloadUrlsChanged(); } diff --git a/libraries/networking/src/ResourceCache.cpp b/libraries/networking/src/ResourceCache.cpp index 11d0f31217..6c74da749c 100644 --- a/libraries/networking/src/ResourceCache.cpp +++ b/libraries/networking/src/ResourceCache.cpp @@ -41,9 +41,9 @@ void ResourceCache::refreshAll() { resetResourceCounters(); // Refresh all remaining resources in use - foreach (auto resource, _resources) { - if (!resource.isNull()) { - resource.data()->refresh(); + foreach (QSharedPointer resource, _resources) { + if (resource) { + resource->refresh(); } } } @@ -110,7 +110,7 @@ QSharedPointer ResourceCache::getResource(const QUrl& url, const QUrl& } if (QThread::currentThread() != thread()) { - assert(delayLoad && !extra); + assert(delayLoad); getResourceAsynchronously(url); return QSharedPointer(); } @@ -208,9 +208,19 @@ void ResourceCacheSharedItems::appendPendingRequest(QWeakPointer resou _pendingRequests.append(resource); } -QList> ResourceCacheSharedItems::getPendingRequests() const { - Lock lock(_mutex); - return _pendingRequests; +QList> ResourceCacheSharedItems::getPendingRequests() { + QList> result; + + { + Lock lock(_mutex); + foreach(QSharedPointer resource, _pendingRequests) { + if (resource) { + result.append(resource); + } + } + _pendingRequests.removeAll(QWeakPointer()); + } + return result; } uint32_t ResourceCacheSharedItems::getPendingRequestsCount() const { @@ -218,9 +228,19 @@ uint32_t ResourceCacheSharedItems::getPendingRequestsCount() const { return _pendingRequests.size(); } -QList> ResourceCacheSharedItems::getLoadingRequests() const { - Lock lock(_mutex); - return _loadingRequests; +QList> ResourceCacheSharedItems::getLoadingRequests() { + QList> result; + + { + Lock lock(_mutex); + foreach(QSharedPointer resource, _loadingRequests) { + if (resource) { + result.append(resource); + } + } + _loadingRequests.removeAll(QWeakPointer()); + } + return result; } void ResourceCacheSharedItems::removeRequest(QWeakPointer resource) { @@ -254,7 +274,7 @@ QSharedPointer ResourceCacheSharedItems::getHighestPendingRequest() { return highestResource; } -const QList> ResourceCache::getLoadingRequests() { +QList> ResourceCache::getLoadingRequests() { return DependencyManager::get()->getLoadingRequests(); } diff --git a/libraries/networking/src/ResourceCache.h b/libraries/networking/src/ResourceCache.h index 034ca74882..94fcafee9e 100644 --- a/libraries/networking/src/ResourceCache.h +++ b/libraries/networking/src/ResourceCache.h @@ -65,9 +65,9 @@ public: void appendPendingRequest(QWeakPointer newRequest); void appendActiveRequest(QWeakPointer newRequest); void removeRequest(QWeakPointer doneRequest); - QList> getPendingRequests() const; + QList> getPendingRequests(); uint32_t getPendingRequestsCount() const; - QList> getLoadingRequests() const; + QList> getLoadingRequests(); QSharedPointer getHighestPendingRequest(); private: @@ -88,8 +88,6 @@ class ResourceCache : public QObject { Q_PROPERTY(size_t sizeCached READ getSizeCachedResources NOTIFY dirty) public: - virtual ~ResourceCache(); - size_t getNumTotalResources() const { return _numTotalResources; } size_t getSizeTotalResources() const { return _totalResourcesSize; } @@ -106,10 +104,12 @@ public: void setUnusedResourceCacheSize(qint64 unusedResourcesMaxSize); qint64 getUnusedResourceCacheSize() const { return _unusedResourcesMaxSize; } - static const QList> getLoadingRequests(); + static QList> getLoadingRequests(); static int getPendingRequestCount(); + ResourceCache(QObject* parent = nullptr); + virtual ~ResourceCache(); void refreshAll(); void refresh(const QUrl& url); @@ -124,8 +124,6 @@ protected slots: void updateTotalSize(const qint64& oldSize, const qint64& newSize); protected: - ResourceCache(QObject* parent = nullptr); - /// Loads a resource from the specified URL. /// \param fallback a fallback URL to load if the desired one is unavailable /// \param delayLoad if true, don't load the resource immediately; wait until load is first requested