Merge pull request #7213 from Atlante45/fix/resource-caching

Fix resource caches overlap

assignment-client/src/Agent.cpp

@@ -55,13 +55,7 @@ Agent::Agent(ReceivedMessage& message) :
 {
     DependencyManager::get<EntityScriptingInterface>()->setPacketSender(&_entityEditSender);
 
-    auto assetClient = DependencyManager::set<AssetClient>();
-
-    QThread* assetThread = new QThread;
-    assetThread->setObjectName("Asset Thread");
-    assetClient->moveToThread(assetThread);
-    connect(assetThread, &QThread::started, assetClient.data(), &AssetClient::init);
-    assetThread->start();
+    ResourceManager::init();
 
     DependencyManager::registerInheritance<SpatialParentFinder, AssignmentParentFinder>();
 
@@ -471,11 +465,7 @@ void Agent::aboutToFinish() {
     // our entity tree is going to go away so tell that to the EntityScriptingInterface
     DependencyManager::get<EntityScriptingInterface>()->setEntityTree(nullptr);
 
-    // cleanup the AssetClient thread
-    QThread* assetThread = DependencyManager::get<AssetClient>()->thread();
-    DependencyManager::destroy<AssetClient>();
-    assetThread->quit();
-    assetThread->wait();
+    ResourceManager::cleanup();
     
     // cleanup the AudioInjectorManager (and any still running injectors)
     DependencyManager::destroy<AudioInjectorManager>();

interface/src/Application.cpp

@@ -370,7 +370,6 @@ bool setupEssentials(int& argc, char** argv) {
     DependencyManager::set<AutoUpdater>();
     DependencyManager::set<PathUtils>();
     DependencyManager::set<InterfaceActionFactory>();
-    DependencyManager::set<AssetClient>();
     DependencyManager::set<AudioInjectorManager>();
     DependencyManager::set<MessagesClient>();
     DependencyManager::set<UserInputMapper>();
@@ -528,13 +527,7 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer) :
 
     audioThread->start();
 
-    // Setup AssetClient
-    auto assetClient = DependencyManager::get<AssetClient>();
-    QThread* assetThread = new QThread;
-    assetThread->setObjectName("Asset Thread");
-    assetClient->moveToThread(assetThread);
-    connect(assetThread, &QThread::started, assetClient.data(), &AssetClient::init);
-    assetThread->start();
+    ResourceManager::init();
 
     // Setup MessagesClient
     auto messagesClient = DependencyManager::get<MessagesClient>();
@@ -644,13 +637,6 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer) :
     connect(&identityPacketTimer, &QTimer::timeout, getMyAvatar(), &MyAvatar::sendIdentityPacket);
     identityPacketTimer.start(AVATAR_IDENTITY_PACKET_SEND_INTERVAL_MSECS);
 
-    QString cachePath = QStandardPaths::writableLocation(QStandardPaths::DataLocation);
-    QNetworkAccessManager& networkAccessManager = NetworkAccessManager::getInstance();
-    QNetworkDiskCache* cache = new QNetworkDiskCache();
-    cache->setMaximumCacheSize(MAXIMUM_CACHE_SIZE);
-    cache->setCacheDirectory(!cachePath.isEmpty() ? cachePath : "interfaceCache");
-    networkAccessManager.setCache(cache);
-
     ResourceCache::setRequestLimit(3);
 
     _glWidget = new GLCanvas();
@@ -1085,13 +1071,6 @@ void Application::cleanupBeforeQuit() {
     DependencyManager::destroy<OffscreenUi>();
 }
 
-void Application::emptyLocalCache() {
-    if (auto cache = NetworkAccessManager::getInstance().cache()) {
-        qDebug() << "DiskCacheEditor::clear(): Clearing disk cache.";
-        cache->clear();
-    }
-}
-
 Application::~Application() {
     EntityTreePointer tree = getEntities()->getTree();
     tree->setSimulation(NULL);
@@ -1129,11 +1108,7 @@ Application::~Application() {
     DependencyManager::destroy<ScriptCache>();
     DependencyManager::destroy<SoundCache>();
 
-    // cleanup the AssetClient thread
-    QThread* assetThread = DependencyManager::get<AssetClient>()->thread();
-    DependencyManager::destroy<AssetClient>();
-    assetThread->quit();
-    assetThread->wait();
+    ResourceManager::cleanup();
 
     QThread* nodeThread = DependencyManager::get<NodeList>()->thread();
 
@@ -3081,7 +3056,7 @@ void Application::reloadResourceCaches() {
     _viewFrustum.setOrientation(glm::quat());
     queryOctree(NodeType::EntityServer, PacketType::EntityQuery, _entityServerJurisdictions);
 
-    emptyLocalCache();
+    DependencyManager::get<AssetClient>()->clearCache();
 
     DependencyManager::get<AnimationCache>()->refreshAll();
     DependencyManager::get<ModelCache>()->refreshAll();

interface/src/Application.h

@@ -330,8 +330,6 @@ private:
 
     void cleanupBeforeQuit();
 
-    void emptyLocalCache();
-
     void update(float deltaTime);
 
     void setPalmData(Hand* hand, const controller::Pose& pose, float deltaTime, HandData::Hand whichHand, float triggerValue);

interface/src/ui/DiskCacheEditor.cpp

@@ -9,23 +9,21 @@
 //  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
 //
 
-#include <functional>
+#include "DiskCacheEditor.h"
 
 #include <QDebug>
 #include <QDialog>
 #include <QGridLayout>
 #include <QPushButton>
 #include <QLabel>
-#include <QNetworkDiskCache>
+#include <QTimer>
 #include <QMessageBox>
 
-#include <NetworkAccessManager.h>
+#include <AssetClient.h>
 
-#include "DiskCacheEditor.h"
 #include "OffscreenUi.h"
 
 DiskCacheEditor::DiskCacheEditor(QWidget* parent) : QObject(parent) {
-    
 }
 
 QWindow* DiskCacheEditor::windowHandle() {
@@ -33,7 +31,6 @@ QWindow* DiskCacheEditor::windowHandle() {
 }
 
 void DiskCacheEditor::toggle() {
-    qDebug() << "DiskCacheEditor::toggle()";
     if (!_dialog) {
         makeDialog();
     }
@@ -88,17 +85,17 @@ void DiskCacheEditor::makeDialog() {
     Q_CHECK_PTR(_maxSize);
     _maxSize->setAlignment(Qt::AlignLeft);
     layout->addWidget(_maxSize, 2, 1, 1, 3);
-    
+
     refresh();
-    
-    
-    QPushButton* refreshCacheButton = new QPushButton(_dialog);
-    Q_CHECK_PTR(refreshCacheButton);
-    refreshCacheButton->setText("Refresh");
-    refreshCacheButton->setToolTip("Reload the cache stats.");
-    connect(refreshCacheButton, SIGNAL(clicked()), SLOT(refresh()));
-    layout->addWidget(refreshCacheButton, 3, 2);
-    
+
+
+    static const int REFRESH_INTERVAL = 100; // msec
+    _refreshTimer = new QTimer(_dialog);
+    _refreshTimer->setInterval(REFRESH_INTERVAL);
+    _refreshTimer->setSingleShot(false);
+    QObject::connect(_refreshTimer.data(), &QTimer::timeout, this, &DiskCacheEditor::refresh);
+    _refreshTimer->start();
+
     QPushButton* clearCacheButton = new QPushButton(_dialog);
     Q_CHECK_PTR(clearCacheButton);
     clearCacheButton->setText("Clear");
@@ -108,7 +105,11 @@ void DiskCacheEditor::makeDialog() {
 }
 
 void DiskCacheEditor::refresh() {
-    static const std::function<QString(qint64)> stringify = [](qint64 number) {
+    DependencyManager::get<AssetClient>()->cacheInfoRequest(this, "cacheInfoCallback");
+}
+
+void DiskCacheEditor::cacheInfoCallback(QString cacheDirectory, qint64 cacheSize, qint64 maximumCacheSize) {
+    static const auto stringify = [](qint64 number) {
         static const QStringList UNITS = QStringList() << "B" << "KB" << "MB" << "GB";
         static const qint64 CHUNK = 1024;
         QString unit;
@@ -122,30 +123,24 @@ void DiskCacheEditor::refresh() {
         }
         return QString("%0 %1").arg(number).arg(UNITS[i]);
     };
-    QNetworkDiskCache* cache = qobject_cast<QNetworkDiskCache*>(NetworkAccessManager::getInstance().cache());
     
     if (_path) {
-        _path->setText(cache->cacheDirectory());
+        _path->setText(cacheDirectory);
     }
     if (_size) {
-        _size->setText(stringify(cache->cacheSize()));
+        _size->setText(stringify(cacheSize));
     }
     if (_maxSize) {
-        _maxSize->setText(stringify(cache->maximumCacheSize()));
+        _maxSize->setText(stringify(maximumCacheSize));
     }
 }
 
 void DiskCacheEditor::clear() {
-    QMessageBox::StandardButton buttonClicked =
-                                    OffscreenUi::question(_dialog, "Clearing disk cache",
-                                              "You are about to erase all the content of the disk cache, "
-                                              "are you sure you want to do that?",
-                                              QMessageBox::Ok | QMessageBox::Cancel);
+    auto buttonClicked = OffscreenUi::question(_dialog, "Clearing disk cache",
+                                               "You are about to erase all the content of the disk cache, "
+                                               "are you sure you want to do that?",
+                                               QMessageBox::Ok | QMessageBox::Cancel);
     if (buttonClicked == QMessageBox::Ok) {
-        if (auto cache = NetworkAccessManager::getInstance().cache()) {
-            qDebug() << "DiskCacheEditor::clear(): Clearing disk cache.";
-            cache->clear();
-        }
+        DependencyManager::get<AssetClient>()->clearCache();
     }
-    refresh();
 }

interface/src/ui/DiskCacheEditor.h

@@ -18,6 +18,7 @@
 class QDialog;
 class QLabel;
 class QWindow;
+class QTimer;
 
 class DiskCacheEditor : public QObject {
     Q_OBJECT
@@ -32,8 +33,9 @@ public slots:
     
 private slots:
     void refresh();
+    void cacheInfoCallback(QString cacheDirectory, qint64 cacheSize, qint64 maximumCacheSize);
     void clear();
-    
+
 private:
     void makeDialog();
     
@@ -41,6 +43,7 @@ private:
     QPointer<QLabel> _path;
     QPointer<QLabel> _size;
     QPointer<QLabel> _maxSize;
+    QPointer<QTimer> _refreshTimer;
 };
 
 #endif // hifi_DiskCacheEditor_h

libraries/networking/src/AssetClient.cpp

@@ -47,22 +47,53 @@ AssetClient::AssetClient() {
 }
 
 void AssetClient::init() {
-    if (QThread::currentThread() != thread()) {
-        QMetaObject::invokeMethod(this, "init", Qt::BlockingQueuedConnection);
-    }
-    
+    Q_ASSERT(QThread::currentThread() == thread());
+
     // Setup disk cache if not already
-    QNetworkAccessManager& networkAccessManager = NetworkAccessManager::getInstance();
+    auto& networkAccessManager = NetworkAccessManager::getInstance();
     if (!networkAccessManager.cache()) {
         QString cachePath = QStandardPaths::writableLocation(QStandardPaths::DataLocation);
         cachePath = !cachePath.isEmpty() ? cachePath : "interfaceCache";
-        
+
         QNetworkDiskCache* cache = new QNetworkDiskCache();
         cache->setMaximumCacheSize(MAXIMUM_CACHE_SIZE);
         cache->setCacheDirectory(cachePath);
         networkAccessManager.setCache(cache);
-        qCDebug(asset_client) << "AssetClient disk cache setup at" << cachePath
-                                << "(size:" << MAXIMUM_CACHE_SIZE / BYTES_PER_GIGABYTES << "GB)";
+        qDebug() << "ResourceManager disk cache setup at" << cachePath
+                 << "(size:" << MAXIMUM_CACHE_SIZE / BYTES_PER_GIGABYTES << "GB)";
+    }
+}
+
+
+void AssetClient::cacheInfoRequest(QObject* reciever, QString slot) {
+    if (QThread::currentThread() != thread()) {
+        QMetaObject::invokeMethod(this, "cacheInfoRequest", Qt::QueuedConnection,
+                                  Q_ARG(QObject*, reciever), Q_ARG(QString, slot));
+        return;
+    }
+
+
+    if (auto* cache = qobject_cast<QNetworkDiskCache*>(NetworkAccessManager::getInstance().cache())) {
+        QMetaObject::invokeMethod(reciever, slot.toStdString().data(), Qt::QueuedConnection,
+                                  Q_ARG(QString, cache->cacheDirectory()),
+                                  Q_ARG(qint64, cache->cacheSize()),
+                                  Q_ARG(qint64, cache->maximumCacheSize()));
+    } else {
+        qCWarning(asset_client) << "No disk cache to get info from.";
+    }
+}
+
+void AssetClient::clearCache() {
+    if (QThread::currentThread() != thread()) {
+        QMetaObject::invokeMethod(this, "clearCache", Qt::QueuedConnection);
+        return;
+    }
+
+    if (auto cache = NetworkAccessManager::getInstance().cache()) {
+        qDebug() << "AssetClient::clearCache(): Clearing disk cache.";
+        cache->clear();
+    } else {
+        qCWarning(asset_client) << "No disk cache to clear.";
     }
 }
 

libraries/networking/src/AssetClient.h

@@ -43,13 +43,17 @@ class AssetClient : public QObject, public Dependency {
     Q_OBJECT
 public:
     AssetClient();
-    
-    Q_INVOKABLE void init();
 
     Q_INVOKABLE AssetRequest* createRequest(const QString& hash, const QString& extension);
     Q_INVOKABLE AssetUpload* createUpload(const QString& filename);
     Q_INVOKABLE AssetUpload* createUpload(const QByteArray& data, const QString& extension);
 
+public slots:
+    void init();
+
+    void cacheInfoRequest(QObject* reciever, QString slot);
+    void clearCache();
+
 private slots:
     void handleAssetGetInfoReply(QSharedPointer<ReceivedMessage> message, SharedNodePointer senderNode);
     void handleAssetGetReply(QSharedPointer<ReceivedMessage> message, SharedNodePointer senderNode);

libraries/networking/src/AssetResourceRequest.h

@@ -20,7 +20,7 @@
 class AssetResourceRequest : public ResourceRequest {
     Q_OBJECT
 public:
-    AssetResourceRequest(QObject* parent, const QUrl& url) : ResourceRequest(parent, url) { }
+    AssetResourceRequest(const QUrl& url) : ResourceRequest(url) { }
     ~AssetResourceRequest();
 
 protected:

libraries/networking/src/FileResourceRequest.h

@@ -19,7 +19,7 @@
 class FileResourceRequest : public ResourceRequest {
     Q_OBJECT
 public:
-    FileResourceRequest(QObject* parent, const QUrl& url) : ResourceRequest(parent, url) { }
+    FileResourceRequest(const QUrl& url) : ResourceRequest(url) { }
 
 protected:
     virtual void doSend() override;

libraries/networking/src/HTTPResourceRequest.cpp

@@ -28,6 +28,25 @@ HTTPResourceRequest::~HTTPResourceRequest() {
     }
 }
 
+void HTTPResourceRequest::setupTimer() {
+    Q_ASSERT(!_sendTimer);
+    static const int TIMEOUT_MS = 10000;
+
+    _sendTimer = new QTimer();
+    connect(this, &QObject::destroyed, _sendTimer, &QTimer::deleteLater);
+    connect(_sendTimer, &QTimer::timeout, this, &HTTPResourceRequest::onTimeout);
+
+    _sendTimer->setSingleShot(true);
+    _sendTimer->start(TIMEOUT_MS);
+}
+
+void HTTPResourceRequest::cleanupTimer() {
+    Q_ASSERT(_sendTimer);
+    _sendTimer->disconnect(this);
+    _sendTimer->deleteLater();
+    _sendTimer = nullptr;
+}
+
 void HTTPResourceRequest::doSend() {
     QNetworkRequest networkRequest(_url);
     networkRequest.setHeader(QNetworkRequest::UserAgentHeader, HIGH_FIDELITY_USER_AGENT);
@@ -42,18 +61,15 @@ void HTTPResourceRequest::doSend() {
     
     connect(_reply, &QNetworkReply::finished, this, &HTTPResourceRequest::onRequestFinished);
     connect(_reply, &QNetworkReply::downloadProgress, this, &HTTPResourceRequest::onDownloadProgress);
-    connect(&_sendTimer, &QTimer::timeout, this, &HTTPResourceRequest::onTimeout);
 
-    static const int TIMEOUT_MS = 10000;
-    _sendTimer.setSingleShot(true);
-    _sendTimer.start(TIMEOUT_MS);
+    setupTimer();
 }
 
 void HTTPResourceRequest::onRequestFinished() {
     Q_ASSERT(_state == InProgress);
     Q_ASSERT(_reply);
 
-    _sendTimer.stop();
+    cleanupTimer();
     
     switch(_reply->error()) {
         case QNetworkReply::NoError:
@@ -80,7 +96,7 @@ void HTTPResourceRequest::onDownloadProgress(qint64 bytesReceived, qint64 bytesT
     Q_ASSERT(_state == InProgress);
     
     // We've received data, so reset the timer
-    _sendTimer.start();
+    _sendTimer->start();
 
     emit progress(bytesReceived, bytesTotal);
 }
@@ -91,6 +107,8 @@ void HTTPResourceRequest::onTimeout() {
     _reply->abort();
     _reply->deleteLater();
     _reply = nullptr;
+
+    cleanupTimer();
     
     _result = Timeout;
     _state = Finished;

libraries/networking/src/HTTPResourceRequest.h

@@ -21,7 +21,7 @@
 class HTTPResourceRequest : public ResourceRequest {
     Q_OBJECT
 public:
-    HTTPResourceRequest(QObject* parent, const QUrl& url) : ResourceRequest(parent, url) { }
+    HTTPResourceRequest(const QUrl& url) : ResourceRequest(url) { }
     ~HTTPResourceRequest();
 
 protected:
@@ -33,7 +33,10 @@ private slots:
     void onRequestFinished();
 
 private:
-    QTimer _sendTimer;
+    void setupTimer();
+    void cleanupTimer();
+
+    QTimer* _sendTimer { nullptr };
     QNetworkReply* _reply { nullptr };
 };
 

libraries/networking/src/ResourceManager.cpp

@@ -11,12 +11,20 @@
 
 #include "ResourceManager.h"
 
-#include "AssetResourceRequest.h"
-#include "FileResourceRequest.h"
-#include "HTTPResourceRequest.h"
+#include <QNetworkDiskCache>
+#include <QStandardPaths>
+#include <QThread>
 
 #include <SharedUtil.h>
 
+
+#include "AssetResourceRequest.h"
+#include "FileResourceRequest.h"
+#include "HTTPResourceRequest.h"
+#include "NetworkAccessManager.h"
+
+
+QThread ResourceManager::_thread;
 ResourceManager::PrefixMap ResourceManager::_prefixMap;
 QMutex ResourceManager::_prefixMapLock;
 
@@ -67,18 +75,41 @@ QUrl ResourceManager::normalizeURL(const QUrl& originalUrl) {
     return url;
 }
 
+void ResourceManager::init() {
+    _thread.setObjectName("Ressource Manager Thread");
+
+    auto assetClient = DependencyManager::set<AssetClient>();
+    assetClient->moveToThread(&_thread);
+    QObject::connect(&_thread, &QThread::started, assetClient.data(), &AssetClient::init);
+
+    _thread.start();
+}
+
+void ResourceManager::cleanup() {
+    // cleanup the AssetClient thread
+    DependencyManager::destroy<AssetClient>();
+    _thread.quit();
+    _thread.wait();
+}
+
 ResourceRequest* ResourceManager::createResourceRequest(QObject* parent, const QUrl& url) {
     auto normalizedURL = normalizeURL(url);
     auto scheme = normalizedURL.scheme();
+
+    ResourceRequest* request = nullptr;
+
     if (scheme == URL_SCHEME_FILE) {
-        return new FileResourceRequest(parent, normalizedURL);
+        request = new FileResourceRequest(normalizedURL);
     } else if (scheme == URL_SCHEME_HTTP || scheme == URL_SCHEME_HTTPS || scheme == URL_SCHEME_FTP) {
-        return new HTTPResourceRequest(parent, normalizedURL);
+        request = new HTTPResourceRequest(normalizedURL);
     } else if (scheme == URL_SCHEME_ATP) {
-        return new AssetResourceRequest(parent, normalizedURL);
+        request = new AssetResourceRequest(normalizedURL);
+    } else {
+        qDebug() << "Unknown scheme (" << scheme << ") for URL: " << url.url();
+        return nullptr;
     }
+    Q_ASSERT(request);
 
-    qDebug() << "Unknown scheme (" << scheme << ") for URL: " << url.url();
-
-    return nullptr;
+    request->moveToThread(&_thread);
+    return request;
 }

libraries/networking/src/ResourceManager.h

@@ -29,8 +29,15 @@ public:
     static void setUrlPrefixOverride(const QString& prefix, const QString& replacement);
     static QString normalizeURL(const QString& urlString);
     static QUrl normalizeURL(const QUrl& url);
+
     static ResourceRequest* createResourceRequest(QObject* parent, const QUrl& url);
+
+    static void init();
+    static void cleanup();
+
 private:
+    static QThread _thread;
+
     using PrefixMap = std::map<QString, QString>;
 
     static PrefixMap _prefixMap;

libraries/networking/src/ResourceRequest.cpp

@@ -11,12 +11,15 @@
 
 #include "ResourceRequest.h"
 
-ResourceRequest::ResourceRequest(QObject* parent, const QUrl& url) :
-    QObject(parent),
-    _url(url) {
-}
+#include <QtCore/QThread>
+
+ResourceRequest::ResourceRequest(const QUrl& url) : _url(url) { }
 
 void ResourceRequest::send() {
+    if (QThread::currentThread() != thread()) {
+        QMetaObject::invokeMethod(this, "send", Qt::QueuedConnection);
+        return;
+    }
     Q_ASSERT(_state == NotStarted);
 
     _state = InProgress;

libraries/networking/src/ResourceRequest.h

@@ -20,7 +20,7 @@
 class ResourceRequest : public QObject {
     Q_OBJECT
 public:
-    ResourceRequest(QObject* parent, const QUrl& url);
+    ResourceRequest(const QUrl& url);
 
     enum State {
         NotStarted = 0,
@@ -38,7 +38,6 @@ public:
         NotFound
     };
 
-    void send();
     QByteArray getData() { return _data; }
     State getState() const { return _state; }
     Result getResult() const { return _result; }
@@ -47,8 +46,11 @@ public:
 
     void setCacheEnabled(bool value) { _cacheEnabled = value; }
 
+public slots:
+    void send();
+
 signals:
-    void progress(uint64_t bytesReceived, uint64_t bytesTotal);
+    void progress(qint64 bytesReceived, qint64 bytesTotal);
     void finished();
 
 protected: