From 2c1762526d3e859b3560db56491a48aec1316cdc Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Thu, 25 Feb 2016 16:44:27 -0800 Subject: [PATCH] Move all resource requests to the same thread --- assignment-client/src/Agent.cpp | 18 ++----- interface/src/Application.cpp | 24 ++------- libraries/networking/src/AssetClient.cpp | 14 +++--- libraries/networking/src/AssetClient.h | 5 +- .../networking/src/AssetResourceRequest.h | 2 +- .../networking/src/FileResourceRequest.h | 2 +- .../networking/src/HTTPResourceRequest.cpp | 30 +++++++++--- .../networking/src/HTTPResourceRequest.h | 7 ++- libraries/networking/src/ResourceManager.cpp | 49 +++++++++++++++---- libraries/networking/src/ResourceManager.h | 7 +++ libraries/networking/src/ResourceRequest.cpp | 11 +++-- libraries/networking/src/ResourceRequest.h | 8 +-- 12 files changed, 106 insertions(+), 71 deletions(-) diff --git a/assignment-client/src/Agent.cpp b/assignment-client/src/Agent.cpp index 3ff7eafd6b..ccf296865a 100644 --- a/assignment-client/src/Agent.cpp +++ b/assignment-client/src/Agent.cpp @@ -55,14 +55,6 @@ Agent::Agent(ReceivedMessage& message) : { DependencyManager::get()->setPacketSender(&_entityEditSender); - auto assetClient = DependencyManager::set(); - - QThread* assetThread = new QThread; - assetThread->setObjectName("Asset Thread"); - assetClient->moveToThread(assetThread); - connect(assetThread, &QThread::started, assetClient.data(), &AssetClient::init); - assetThread->start(); - DependencyManager::registerInheritance(); DependencyManager::set(); @@ -81,6 +73,8 @@ Agent::Agent(ReceivedMessage& message) : { PacketType::OctreeStats, PacketType::EntityData, PacketType::EntityErase }, this, "handleOctreePacket"); packetReceiver.registerListener(PacketType::Jurisdiction, this, "handleJurisdictionPacket"); + + ResourceManager::init(); } void Agent::handleOctreePacket(QSharedPointer message, SharedNodePointer senderNode) { @@ -470,13 +464,9 @@ void Agent::aboutToFinish() { // our entity tree is going to go away so tell that to the EntityScriptingInterface DependencyManager::get()->setEntityTree(nullptr); - - // cleanup the AssetClient thread - QThread* assetThread = DependencyManager::get()->thread(); - DependencyManager::destroy(); - assetThread->quit(); - assetThread->wait(); // cleanup the AudioInjectorManager (and any still running injectors) DependencyManager::destroy(); + + ResourceManager::cleanup(); } diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 15b1836173..8e2b2175fe 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -370,7 +370,6 @@ bool setupEssentials(int& argc, char** argv) { DependencyManager::set(); DependencyManager::set(); DependencyManager::set(); - DependencyManager::set(); DependencyManager::set(); DependencyManager::set(); DependencyManager::set(); @@ -528,13 +527,7 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer) : audioThread->start(); - // Setup AssetClient - auto assetClient = DependencyManager::get(); - 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(); @@ -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(); @@ -1129,12 +1115,6 @@ Application::~Application() { DependencyManager::destroy(); DependencyManager::destroy(); - // cleanup the AssetClient thread - QThread* assetThread = DependencyManager::get()->thread(); - DependencyManager::destroy(); - assetThread->quit(); - assetThread->wait(); - QThread* nodeThread = DependencyManager::get()->thread(); // remove the NodeList from the DependencyManager @@ -1150,6 +1130,8 @@ Application::~Application() { ConnexionClient::getInstance().destroy(); #endif + ResourceManager::cleanup(); + qInstallMessageHandler(NULL); // NOTE: Do this as late as possible so we continue to get our log messages } diff --git a/libraries/networking/src/AssetClient.cpp b/libraries/networking/src/AssetClient.cpp index 4e7e7f24ba..203d4d9ba6 100644 --- a/libraries/networking/src/AssetClient.cpp +++ b/libraries/networking/src/AssetClient.cpp @@ -47,22 +47,20 @@ 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)"; } } diff --git a/libraries/networking/src/AssetClient.h b/libraries/networking/src/AssetClient.h index 58790ef926..67ce4daa8f 100644 --- a/libraries/networking/src/AssetClient.h +++ b/libraries/networking/src/AssetClient.h @@ -43,13 +43,14 @@ 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(); + private slots: void handleAssetGetInfoReply(QSharedPointer message, SharedNodePointer senderNode); void handleAssetGetReply(QSharedPointer message, SharedNodePointer senderNode); diff --git a/libraries/networking/src/AssetResourceRequest.h b/libraries/networking/src/AssetResourceRequest.h index 0024426be0..4d2ea56ca4 100644 --- a/libraries/networking/src/AssetResourceRequest.h +++ b/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: diff --git a/libraries/networking/src/FileResourceRequest.h b/libraries/networking/src/FileResourceRequest.h index 4ff0d2ecf2..547b754cb5 100644 --- a/libraries/networking/src/FileResourceRequest.h +++ b/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; diff --git a/libraries/networking/src/HTTPResourceRequest.cpp b/libraries/networking/src/HTTPResourceRequest.cpp index 34c9c1dad2..11ab436933 100644 --- a/libraries/networking/src/HTTPResourceRequest.cpp +++ b/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; diff --git a/libraries/networking/src/HTTPResourceRequest.h b/libraries/networking/src/HTTPResourceRequest.h index f0d3bb82d9..cc628d8855 100644 --- a/libraries/networking/src/HTTPResourceRequest.h +++ b/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 }; }; diff --git a/libraries/networking/src/ResourceManager.cpp b/libraries/networking/src/ResourceManager.cpp index f195011290..1d41266f3a 100644 --- a/libraries/networking/src/ResourceManager.cpp +++ b/libraries/networking/src/ResourceManager.cpp @@ -11,12 +11,20 @@ #include "ResourceManager.h" -#include "AssetResourceRequest.h" -#include "FileResourceRequest.h" -#include "HTTPResourceRequest.h" +#include +#include +#include #include + +#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->moveToThread(&_thread); + QObject::connect(&_thread, &QThread::started, assetClient.data(), &AssetClient::init); + + _thread.start(); +} + +void ResourceManager::cleanup() { + // cleanup the AssetClient thread + DependencyManager::destroy(); + _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; } diff --git a/libraries/networking/src/ResourceManager.h b/libraries/networking/src/ResourceManager.h index c010c67f9b..162892abaf 100644 --- a/libraries/networking/src/ResourceManager.h +++ b/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; static PrefixMap _prefixMap; diff --git a/libraries/networking/src/ResourceRequest.cpp b/libraries/networking/src/ResourceRequest.cpp index c6880636ea..e6402d6b25 100644 --- a/libraries/networking/src/ResourceRequest.cpp +++ b/libraries/networking/src/ResourceRequest.cpp @@ -11,12 +11,15 @@ #include "ResourceRequest.h" -ResourceRequest::ResourceRequest(QObject* parent, const QUrl& url) : - QObject(parent), - _url(url) { -} +#include + +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; diff --git a/libraries/networking/src/ResourceRequest.h b/libraries/networking/src/ResourceRequest.h index b229951a49..f940221d9d 100644 --- a/libraries/networking/src/ResourceRequest.h +++ b/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: