Make sure we don't use raw resource ptr

This commit is contained in:
Atlante45 2016-04-10 21:54:49 -07:00
parent b952147fca
commit afdfef1482
8 changed files with 71 additions and 58 deletions

View file

@ -122,8 +122,8 @@ void DownloadInfoResultFromScriptValue(const QScriptValue& object, DownloadInfoR
DownloadInfoResult GlobalServicesScriptingInterface::getDownloadInfo() { DownloadInfoResult GlobalServicesScriptingInterface::getDownloadInfo() {
DownloadInfoResult result; DownloadInfoResult result;
foreach(Resource* resource, ResourceCache::getLoadingRequests()) { foreach(QSharedPointer<Resource> resource, ResourceCache::getLoadingRequests()) {
result.downloading.append(resource->getProgress() * 100.0f); result.downloading.append(resource ? (resource->getProgress() * 100.0f) : 0.0f);
} }
result.pending = ResourceCache::getPendingRequestCount(); result.pending = ResourceCache::getPendingRequestCount();
return result; return result;

View file

@ -196,7 +196,7 @@ void Stats::updateStats(bool force) {
STAT_UPDATE(audioMixerPps, -1); STAT_UPDATE(audioMixerPps, -1);
} }
QList<Resource*> loadingRequests = ResourceCache::getLoadingRequests(); QList<QWeakPointer<Resource>> loadingRequests = ResourceCache::getLoadingRequests();
STAT_UPDATE(downloads, loadingRequests.size()); STAT_UPDATE(downloads, loadingRequests.size());
STAT_UPDATE(downloadLimit, ResourceCache::getRequestLimit()) STAT_UPDATE(downloadLimit, ResourceCache::getRequestLimit())
STAT_UPDATE(downloadsPending, ResourceCache::getPendingRequestCount()); STAT_UPDATE(downloadsPending, ResourceCache::getPendingRequestCount());
@ -205,7 +205,8 @@ void Stats::updateStats(bool force) {
bool shouldUpdateUrls = _downloads != _downloadUrls.size(); bool shouldUpdateUrls = _downloads != _downloadUrls.size();
if (!shouldUpdateUrls) { if (!shouldUpdateUrls) {
for (int i = 0; i < _downloads; i++) { for (int i = 0; i < _downloads; i++) {
if (loadingRequests[i]->getURL().toString() != _downloadUrls[i]) { auto request = loadingRequests[i].lock();
if (!request || request->getURL().toString() != _downloadUrls[i]) {
shouldUpdateUrls = true; shouldUpdateUrls = true;
break; break;
} }
@ -214,8 +215,10 @@ void Stats::updateStats(bool force) {
// If the urls have changed, update the list // If the urls have changed, update the list
if (shouldUpdateUrls) { if (shouldUpdateUrls) {
_downloadUrls.clear(); _downloadUrls.clear();
foreach (Resource* resource, loadingRequests) { foreach (QSharedPointer<Resource> request, loadingRequests) {
_downloadUrls << resource->getURL().toString(); if (request) {
_downloadUrls << request->getURL().toString();
}
} }
emit downloadUrlsChanged(); emit downloadUrlsChanged();
} }

View file

@ -568,12 +568,13 @@ bool processStateMachineNode(AnimNode::Pointer node, const QJsonObject& jsonObj,
} }
AnimNodeLoader::AnimNodeLoader(const QUrl& url) : AnimNodeLoader::AnimNodeLoader(const QUrl& url) :
_url(url), _url(url)
_resource(nullptr) { {
_resource = QSharedPointer<Resource>::create(url);
_resource = new Resource(url); _resource->setSelf(_resource);
connect(_resource, &Resource::loaded, this, &AnimNodeLoader::onRequestDone); connect(_resource.data(), &Resource::loaded, this, &AnimNodeLoader::onRequestDone);
connect(_resource, &Resource::failed, this, &AnimNodeLoader::onRequestError); connect(_resource.data(), &Resource::failed, this, &AnimNodeLoader::onRequestError);
_resource->ensureLoading();
} }
AnimNode::Pointer AnimNodeLoader::load(const QByteArray& contents, const QUrl& jsonUrl) { AnimNode::Pointer AnimNodeLoader::load(const QByteArray& contents, const QUrl& jsonUrl) {

View file

@ -41,7 +41,7 @@ protected slots:
protected: protected:
QUrl _url; QUrl _url;
Resource* _resource; QSharedPointer<Resource> _resource;
private: private:
// no copies // no copies

View file

@ -110,7 +110,7 @@ QSharedPointer<Resource> ResourceCache::getResource(const QUrl& url, const QUrl&
} }
if (QThread::currentThread() != thread()) { if (QThread::currentThread() != thread()) {
assert(delayLoad); assert(delayLoad && !extra);
getResourceAsynchronously(url); getResourceAsynchronously(url);
return QSharedPointer<Resource>(); return QSharedPointer<Resource>();
} }
@ -198,17 +198,17 @@ void ResourceCache::updateTotalSize(const qint64& oldSize, const qint64& newSize
emit dirty(); emit dirty();
} }
void ResourceCacheSharedItems::appendActiveRequest(Resource* resource) { void ResourceCacheSharedItems::appendActiveRequest(QWeakPointer<Resource> resource) {
Lock lock(_mutex); Lock lock(_mutex);
_loadingRequests.append(resource); _loadingRequests.append(resource);
} }
void ResourceCacheSharedItems::appendPendingRequest(Resource* resource) { void ResourceCacheSharedItems::appendPendingRequest(QWeakPointer<Resource> resource) {
Lock lock(_mutex); Lock lock(_mutex);
_pendingRequests.append(resource); _pendingRequests.append(resource);
} }
QList<QPointer<Resource>> ResourceCacheSharedItems::getPendingRequests() const { QList<QWeakPointer<Resource>> ResourceCacheSharedItems::getPendingRequests() const {
Lock lock(_mutex); Lock lock(_mutex);
return _pendingRequests; return _pendingRequests;
} }
@ -218,23 +218,24 @@ uint32_t ResourceCacheSharedItems::getPendingRequestsCount() const {
return _pendingRequests.size(); return _pendingRequests.size();
} }
QList<Resource*> ResourceCacheSharedItems::getLoadingRequests() const { QList<QWeakPointer<Resource>> ResourceCacheSharedItems::getLoadingRequests() const {
Lock lock(_mutex); Lock lock(_mutex);
return _loadingRequests; return _loadingRequests;
} }
void ResourceCacheSharedItems::removeRequest(Resource* resource) { void ResourceCacheSharedItems::removeRequest(QWeakPointer<Resource> resource) {
Lock lock(_mutex); Lock lock(_mutex);
_loadingRequests.removeOne(resource); _loadingRequests.removeAll(resource);
} }
Resource* ResourceCacheSharedItems::getHighestPendingRequest() { QSharedPointer<Resource> ResourceCacheSharedItems::getHighestPendingRequest() {
Lock lock(_mutex); Lock lock(_mutex);
// look for the highest priority pending request // look for the highest priority pending request
int highestIndex = -1; int highestIndex = -1;
float highestPriority = -FLT_MAX; float highestPriority = -FLT_MAX;
QSharedPointer<Resource> highestResource;
for (int i = 0; i < _pendingRequests.size();) { for (int i = 0; i < _pendingRequests.size();) {
Resource* resource = _pendingRequests.at(i).data(); auto resource = _pendingRequests.at(i).lock();
if (!resource) { if (!resource) {
_pendingRequests.removeAt(i); _pendingRequests.removeAt(i);
continue; continue;
@ -243,16 +244,25 @@ Resource* ResourceCacheSharedItems::getHighestPendingRequest() {
if (priority >= highestPriority) { if (priority >= highestPriority) {
highestPriority = priority; highestPriority = priority;
highestIndex = i; highestIndex = i;
highestResource = resource;
} }
i++; i++;
} }
if (highestIndex >= 0) { if (highestIndex >= 0) {
return _pendingRequests.takeAt(highestIndex); _pendingRequests.takeAt(highestIndex);
} }
return nullptr; return highestResource;
} }
bool ResourceCache::attemptRequest(Resource* resource) { const QList<QWeakPointer<Resource>> ResourceCache::getLoadingRequests() {
return DependencyManager::get<ResourceCacheSharedItems>()->getLoadingRequests();
}
int ResourceCache::getPendingRequestCount() {
return DependencyManager::get<ResourceCacheSharedItems>()->getPendingRequestsCount();
}
bool ResourceCache::attemptRequest(QSharedPointer<Resource> resource) {
auto sharedItems = DependencyManager::get<ResourceCacheSharedItems>(); auto sharedItems = DependencyManager::get<ResourceCacheSharedItems>();
if (_requestsActive >= _requestLimit) { if (_requestsActive >= _requestLimit) {
@ -267,7 +277,7 @@ bool ResourceCache::attemptRequest(Resource* resource) {
return true; return true;
} }
void ResourceCache::requestCompleted(Resource* resource) { void ResourceCache::requestCompleted(QSharedPointer<Resource> resource) {
auto sharedItems = DependencyManager::get<ResourceCacheSharedItems>(); auto sharedItems = DependencyManager::get<ResourceCacheSharedItems>();
sharedItems->removeRequest(resource); sharedItems->removeRequest(resource);
--_requestsActive; --_requestsActive;
@ -291,11 +301,6 @@ Resource::Resource(const QUrl& url, bool delayLoad) :
_request(nullptr) { _request(nullptr) {
init(); init();
// start loading immediately unless instructed otherwise
if (!(_startedLoading || delayLoad)) {
QTimer::singleShot(0, this, &Resource::ensureLoading);
}
} }
Resource::~Resource() { Resource::~Resource() {
@ -303,7 +308,7 @@ Resource::~Resource() {
_request->disconnect(this); _request->disconnect(this);
_request->deleteLater(); _request->deleteLater();
_request = nullptr; _request = nullptr;
ResourceCache::requestCompleted(this); ResourceCache::requestCompleted(_self);
} }
} }
@ -356,7 +361,7 @@ void Resource::refresh() {
_request->disconnect(this); _request->disconnect(this);
_request->deleteLater(); _request->deleteLater();
_request = nullptr; _request = nullptr;
ResourceCache::requestCompleted(this); ResourceCache::requestCompleted(_self);
} }
init(); init();
@ -401,7 +406,7 @@ void Resource::init() {
void Resource::attemptRequest() { void Resource::attemptRequest() {
_startedLoading = true; _startedLoading = true;
ResourceCache::attemptRequest(this); ResourceCache::attemptRequest(_self);
} }
void Resource::finishedLoading(bool success) { void Resource::finishedLoading(bool success) {
@ -433,7 +438,7 @@ void Resource::makeRequest() {
if (!_request) { if (!_request) {
qCDebug(networking).noquote() << "Failed to get request for" << _url.toDisplayString(); qCDebug(networking).noquote() << "Failed to get request for" << _url.toDisplayString();
ResourceCache::requestCompleted(this); ResourceCache::requestCompleted(_self);
finishedLoading(false); finishedLoading(false);
return; return;
} }
@ -465,7 +470,7 @@ void Resource::handleReplyFinished() {
return; return;
} }
ResourceCache::requestCompleted(this); ResourceCache::requestCompleted(_self);
auto result = _request->getResult(); auto result = _request->getResult();
if (result == ResourceRequest::Success) { if (result == ResourceRequest::Success) {

View file

@ -62,21 +62,20 @@ class ResourceCacheSharedItems : public Dependency {
using Mutex = std::mutex; using Mutex = std::mutex;
using Lock = std::unique_lock<Mutex>; using Lock = std::unique_lock<Mutex>;
public: public:
void appendPendingRequest(Resource* newRequest); void appendPendingRequest(QWeakPointer<Resource> newRequest);
void appendActiveRequest(Resource* newRequest); void appendActiveRequest(QWeakPointer<Resource> newRequest);
void removeRequest(Resource* doneRequest); void removeRequest(QWeakPointer<Resource> doneRequest);
QList<QPointer<Resource>> getPendingRequests() const; QList<QWeakPointer<Resource>> getPendingRequests() const;
uint32_t getPendingRequestsCount() const; uint32_t getPendingRequestsCount() const;
QList<Resource*> getLoadingRequests() const; QList<QWeakPointer<Resource>> getLoadingRequests() const;
Resource* getHighestPendingRequest(); QSharedPointer<Resource> getHighestPendingRequest();
private: private:
ResourceCacheSharedItems() { } ResourceCacheSharedItems() = default;
virtual ~ResourceCacheSharedItems() { }
mutable Mutex _mutex; mutable Mutex _mutex;
QList<QPointer<Resource>> _pendingRequests; QList<QWeakPointer<Resource>> _pendingRequests;
QList<Resource*> _loadingRequests; QList<QWeakPointer<Resource>> _loadingRequests;
}; };
@ -89,6 +88,8 @@ class ResourceCache : public QObject {
Q_PROPERTY(size_t sizeCached READ getSizeCachedResources NOTIFY dirty) Q_PROPERTY(size_t sizeCached READ getSizeCachedResources NOTIFY dirty)
public: public:
virtual ~ResourceCache();
size_t getNumTotalResources() const { return _numTotalResources; } size_t getNumTotalResources() const { return _numTotalResources; }
size_t getSizeTotalResources() const { return _totalResourcesSize; } size_t getSizeTotalResources() const { return _totalResourcesSize; }
@ -105,14 +106,10 @@ public:
void setUnusedResourceCacheSize(qint64 unusedResourcesMaxSize); void setUnusedResourceCacheSize(qint64 unusedResourcesMaxSize);
qint64 getUnusedResourceCacheSize() const { return _unusedResourcesMaxSize; } qint64 getUnusedResourceCacheSize() const { return _unusedResourcesMaxSize; }
static const QList<Resource*> getLoadingRequests() static const QList<QWeakPointer<Resource>> getLoadingRequests();
{ return DependencyManager::get<ResourceCacheSharedItems>()->getLoadingRequests(); }
static int getPendingRequestCount() static int getPendingRequestCount();
{ return DependencyManager::get<ResourceCacheSharedItems>()->getPendingRequestsCount(); }
ResourceCache(QObject* parent = NULL);
virtual ~ResourceCache();
void refreshAll(); void refreshAll();
void refresh(const QUrl& url); void refresh(const QUrl& url);
@ -127,6 +124,8 @@ protected slots:
void updateTotalSize(const qint64& oldSize, const qint64& newSize); void updateTotalSize(const qint64& oldSize, const qint64& newSize);
protected: protected:
ResourceCache(QObject* parent = nullptr);
/// Loads a resource from the specified URL. /// Loads a resource from the specified URL.
/// \param fallback a fallback URL to load if the desired one is unavailable /// \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 /// \param delayLoad if true, don't load the resource immediately; wait until load is first requested
@ -143,8 +142,8 @@ protected:
/// Attempt to load a resource if requests are below the limit, otherwise queue the resource for loading /// Attempt to load a resource if requests are below the limit, otherwise queue the resource for loading
/// \return true if the resource began loading, otherwise false if the resource is in the pending queue /// \return true if the resource began loading, otherwise false if the resource is in the pending queue
static bool attemptRequest(Resource* resource); static bool attemptRequest(QSharedPointer<Resource> resource);
static void requestCompleted(Resource* resource); static void requestCompleted(QSharedPointer<Resource> resource);
static bool attemptHighestPriorityRequest(); static bool attemptHighestPriorityRequest();
private: private:

View file

@ -11,7 +11,6 @@
#include <glm/gtx/transform.hpp> #include <glm/gtx/transform.hpp>
#include <AnimNodeLoader.h>
#include <AnimInverseKinematics.h> #include <AnimInverseKinematics.h>
#include <AnimBlendLinear.h> #include <AnimBlendLinear.h>
#include <AnimationLogging.h> #include <AnimationLogging.h>

View file

@ -34,7 +34,7 @@ void ResourceTests::initTestCase() {
networkAccessManager.setCache(cache); networkAccessManager.setCache(cache);
} }
static Resource* resource = nullptr; static QSharedPointer<Resource> resource;
static bool waitForSignal(QObject *sender, const char *signal, int timeout = 1000) { static bool waitForSignal(QObject *sender, const char *signal, int timeout = 1000) {
@ -55,7 +55,8 @@ void ResourceTests::downloadFirst() {
// download the Mery fst file // download the Mery fst file
QUrl meryUrl = QUrl("http://hifi-public.s3.amazonaws.com/marketplace/contents/e21c0b95-e502-4d15-8c41-ea2fc40f1125/3585ddf674869a67d31d5964f7b52de1.fst"); QUrl meryUrl = QUrl("http://hifi-public.s3.amazonaws.com/marketplace/contents/e21c0b95-e502-4d15-8c41-ea2fc40f1125/3585ddf674869a67d31d5964f7b52de1.fst");
resource = new Resource(meryUrl, false); resource = QSharedPointer<Resource>::create(meryUrl, false);
resource->setSelf(resource);
const int timeout = 1000; const int timeout = 1000;
QEventLoop loop; QEventLoop loop;
@ -67,6 +68,8 @@ void ResourceTests::downloadFirst() {
loop.connect(resource, SIGNAL(failed(QNetworkReply::NetworkError)), SLOT(quit())); loop.connect(resource, SIGNAL(failed(QNetworkReply::NetworkError)), SLOT(quit()));
loop.connect(&timer, SIGNAL(timeout()), SLOT(quit())); loop.connect(&timer, SIGNAL(timeout()), SLOT(quit()));
timer.start(); timer.start();
resource->ensureLoading();
loop.exec(); loop.exec();
QVERIFY(resource->isLoaded()); QVERIFY(resource->isLoaded());
@ -76,7 +79,8 @@ void ResourceTests::downloadAgain() {
// download the Mery fst file // download the Mery fst file
QUrl meryUrl = QUrl("http://hifi-public.s3.amazonaws.com/marketplace/contents/e21c0b95-e502-4d15-8c41-ea2fc40f1125/3585ddf674869a67d31d5964f7b52de1.fst"); QUrl meryUrl = QUrl("http://hifi-public.s3.amazonaws.com/marketplace/contents/e21c0b95-e502-4d15-8c41-ea2fc40f1125/3585ddf674869a67d31d5964f7b52de1.fst");
resource = new Resource(meryUrl, false); resource = QSharedPointer<Resource>::create(meryUrl, false);
resource->setSelf(resource);
const int timeout = 1000; const int timeout = 1000;
QEventLoop loop; QEventLoop loop;
@ -88,6 +92,8 @@ void ResourceTests::downloadAgain() {
loop.connect(resource, SIGNAL(failed(QNetworkReply::NetworkError)), SLOT(quit())); loop.connect(resource, SIGNAL(failed(QNetworkReply::NetworkError)), SLOT(quit()));
loop.connect(&timer, SIGNAL(timeout()), SLOT(quit())); loop.connect(&timer, SIGNAL(timeout()), SLOT(quit()));
timer.start(); timer.start();
resource->ensureLoading();
loop.exec(); loop.exec();
QVERIFY(resource->isLoaded()); QVERIFY(resource->isLoaded());