From 16e6d9a4f9f0ae141890ce7301ac6766914ebea2 Mon Sep 17 00:00:00 2001
From: Bradley Austin Davis <bdavis@saintandreas.org>
Date: Thu, 17 Mar 2016 17:43:52 -0700
Subject: [PATCH] Make shared lists thread safe

---
 libraries/networking/src/ResourceCache.cpp | 101 ++++++++++++++-------
 libraries/networking/src/ResourceCache.h   |  24 ++++-
 2 files changed, 89 insertions(+), 36 deletions(-)

diff --git a/libraries/networking/src/ResourceCache.cpp b/libraries/networking/src/ResourceCache.cpp
index e345bed81c..86b4b74de4 100644
--- a/libraries/networking/src/ResourceCache.cpp
+++ b/libraries/networking/src/ResourceCache.cpp
@@ -159,44 +159,45 @@ void ResourceCache::clearUnusedResource() {
     }
 }
 
-bool ResourceCache::attemptRequest(Resource* resource) {
-    auto sharedItems = DependencyManager::get<ResourceCacheSharedItems>();
-
-    // Disable request limiting for ATP
-    if (resource->getURL().scheme() != URL_SCHEME_ATP) {
-        if (_requestsActive >= _requestLimit) {
-            // wait until a slot becomes available
-            sharedItems->_pendingRequests.append(resource);
-            return false;
-        }
-
-        ++_requestsActive;
-    }	
-
-    sharedItems->_loadingRequests.append(resource);
-    resource->makeRequest();
-    return true;
+void ResourceCacheSharedItems::appendActiveRequest(Resource* resource) {
+    Lock lock(_mutex);
+    _loadingRequests.append(resource);
 }
 
-void ResourceCache::requestCompleted(Resource* resource) {
-    auto sharedItems = DependencyManager::get<ResourceCacheSharedItems>();
-    sharedItems->_loadingRequests.removeOne(resource);
-    if (resource->getURL().scheme() != URL_SCHEME_ATP) {
-        --_requestsActive;
-    }
-
-    attemptHighestPriorityRequest();
+void ResourceCacheSharedItems::appendPendingRequest(Resource* resource) {
+    Lock lock(_mutex);
+    _pendingRequests.append(resource);
 }
 
-bool ResourceCache::attemptHighestPriorityRequest() {
-    auto sharedItems = DependencyManager::get<ResourceCacheSharedItems>();
+QList<QPointer<Resource>> ResourceCacheSharedItems::getPendingRequests() const {
+    Lock lock(_mutex);
+    return _pendingRequests;
+}
+
+uint32_t ResourceCacheSharedItems::getPendingRequestsCount() const {
+    Lock lock(_mutex);
+    return _pendingRequests.size();
+}
+
+QList<Resource*> ResourceCacheSharedItems::getLoadingRequests() const {
+    Lock lock(_mutex);
+    return _loadingRequests;
+}
+
+void ResourceCacheSharedItems::removeRequest(Resource* resource) {
+    Lock lock(_mutex);
+    _loadingRequests.removeOne(resource);
+}
+
+Resource* ResourceCacheSharedItems::getHighestPendingRequest() {
+    Lock lock(_mutex);
     // look for the highest priority pending request
     int highestIndex = -1;
     float highestPriority = -FLT_MAX;
-    for (int i = 0; i < sharedItems->_pendingRequests.size(); ) {
-        Resource* resource = sharedItems->_pendingRequests.at(i).data();
+    for (int i = 0; i < _pendingRequests.size();) {
+        Resource* resource = _pendingRequests.at(i).data();
         if (!resource) {
-            sharedItems->_pendingRequests.removeAt(i);
+            _pendingRequests.removeAt(i);
             continue;
         }
         float priority = resource->getLoadPriority();
@@ -206,7 +207,45 @@ bool ResourceCache::attemptHighestPriorityRequest() {
         }
         i++;
     }
-    return (highestIndex >= 0) && attemptRequest(sharedItems->_pendingRequests.takeAt(highestIndex));
+    if (highestIndex >= 0) {
+        return _pendingRequests.takeAt(highestIndex);
+    }
+    return nullptr;
+}
+
+bool ResourceCache::attemptRequest(Resource* resource) {
+    auto sharedItems = DependencyManager::get<ResourceCacheSharedItems>();
+
+    // Disable request limiting for ATP
+    if (resource->getURL().scheme() != URL_SCHEME_ATP) {
+        if (_requestsActive >= _requestLimit) {
+            // wait until a slot becomes available
+            sharedItems->appendPendingRequest(resource);
+            return false;
+        }
+
+        ++_requestsActive;
+    }
+
+    sharedItems->appendActiveRequest(resource);
+    resource->makeRequest();
+    return true;
+}
+
+void ResourceCache::requestCompleted(Resource* resource) {
+    auto sharedItems = DependencyManager::get<ResourceCacheSharedItems>();
+    sharedItems->removeRequest(resource);
+    if (resource->getURL().scheme() != URL_SCHEME_ATP) {
+        --_requestsActive;
+    }
+
+    attemptHighestPriorityRequest();
+}
+
+bool ResourceCache::attemptHighestPriorityRequest() {
+    auto sharedItems = DependencyManager::get<ResourceCacheSharedItems>();
+    auto resource = sharedItems->getHighestPendingRequest();
+    return (resource && attemptRequest(resource));
 }
 
 const int DEFAULT_REQUEST_LIMIT = 10;
diff --git a/libraries/networking/src/ResourceCache.h b/libraries/networking/src/ResourceCache.h
index a8ab51b7f7..ed938f6cf4 100644
--- a/libraries/networking/src/ResourceCache.h
+++ b/libraries/networking/src/ResourceCache.h
@@ -12,6 +12,7 @@
 #ifndef hifi_ResourceCache_h
 #define hifi_ResourceCache_h
 
+#include <mutex>
 #include <QtCore/QHash>
 #include <QtCore/QList>
 #include <QtCore/QObject>
@@ -53,12 +54,25 @@ static const qint64 MAX_UNUSED_MAX_SIZE = 10 * BYTES_PER_GIGABYTES;
 // object instead
 class ResourceCacheSharedItems : public Dependency  {
     SINGLETON_DEPENDENCY
+
+    using Mutex = std::mutex;
+    using Lock = std::unique_lock<Mutex>;
 public:
-    QList<QPointer<Resource>> _pendingRequests;
-    QList<Resource*> _loadingRequests;
+    void appendPendingRequest(Resource* newRequest);
+    void appendActiveRequest(Resource* newRequest);
+    void removeRequest(Resource* doneRequest);
+    QList<QPointer<Resource>> getPendingRequests() const;
+    uint32_t getPendingRequestsCount() const;
+    QList<Resource*> getLoadingRequests() const;
+    Resource* getHighestPendingRequest();
+
 private:
     ResourceCacheSharedItems() { }
     virtual ~ResourceCacheSharedItems() { }
+
+    mutable Mutex _mutex;
+    QList<QPointer<Resource>> _pendingRequests;
+    QList<Resource*> _loadingRequests;
 };
 
 
@@ -75,11 +89,11 @@ public:
     void setUnusedResourceCacheSize(qint64 unusedResourcesMaxSize);
     qint64 getUnusedResourceCacheSize() const { return _unusedResourcesMaxSize; }
 
-    static const QList<Resource*>& getLoadingRequests() 
-        { return DependencyManager::get<ResourceCacheSharedItems>()->_loadingRequests; }
+    static const QList<Resource*> getLoadingRequests() 
+        { return DependencyManager::get<ResourceCacheSharedItems>()->getLoadingRequests(); }
 
     static int getPendingRequestCount() 
-        { return DependencyManager::get<ResourceCacheSharedItems>()->_pendingRequests.size(); }
+        { return DependencyManager::get<ResourceCacheSharedItems>()->getPendingRequestsCount(); }
 
     ResourceCache(QObject* parent = NULL);
     virtual ~ResourceCache();