Merge pull request #7633 from Atlante45/fix/shared-resource

Fix raw pointers in shared resource cache items
This commit is contained in:
Zach Pomerantz 2016-04-14 17:52:04 -07:00
commit f803ecd2e7
10 changed files with 108 additions and 67 deletions

View file

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

View file

@ -196,7 +196,7 @@ void Stats::updateStats(bool force) {
STAT_UPDATE(audioMixerPps, -1);
}
QList<Resource*> loadingRequests = ResourceCache::getLoadingRequests();
auto loadingRequests = ResourceCache::getLoadingRequests();
STAT_UPDATE(downloads, loadingRequests.size());
STAT_UPDATE(downloadLimit, ResourceCache::getRequestLimit())
STAT_UPDATE(downloadsPending, ResourceCache::getPendingRequestCount());
@ -214,7 +214,7 @@ void Stats::updateStats(bool force) {
// If the urls have changed, update the list
if (shouldUpdateUrls) {
_downloadUrls.clear();
foreach (Resource* resource, loadingRequests) {
foreach (const auto& resource, loadingRequests) {
_downloadUrls << resource->getURL().toString();
}
emit downloadUrlsChanged();

View file

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

View file

@ -13,9 +13,9 @@
#include <memory>
#include <QNetworkReply>
#include <QString>
#include <QUrl>
#include <QtNetwork/QNetworkReply>
#include "AnimNode.h"
@ -41,7 +41,8 @@ protected slots:
protected:
QUrl _url;
Resource* _resource;
QSharedPointer<Resource> _resource;
private:
// no copies

View file

@ -46,6 +46,7 @@ private slots:
private:
GeometryResource::Pointer _geometryResource;
QMetaObject::Connection _connection;
};
void GeometryMappingResource::downloadFinished(const QByteArray& data) {
@ -77,21 +78,28 @@ void GeometryMappingResource::downloadFinished(const QByteArray& data) {
if (_geometryResource->isLoaded()) {
onGeometryMappingLoaded(!_geometryResource->getURL().isEmpty());
} else {
connect(_geometryResource.data(), &Resource::finished, this, &GeometryMappingResource::onGeometryMappingLoaded);
if (_connection) {
disconnect(_connection);
}
_connection = connect(_geometryResource.data(), &Resource::finished,
this, &GeometryMappingResource::onGeometryMappingLoaded);
}
}
}
void GeometryMappingResource::onGeometryMappingLoaded(bool success) {
if (success) {
if (success && _geometryResource) {
_geometry = _geometryResource->_geometry;
_shapes = _geometryResource->_shapes;
_meshes = _geometryResource->_meshes;
_materials = _geometryResource->_materials;
}
// Avoid holding onto extra references
_geometryResource.reset();
// Avoid holding onto extra references
_geometryResource.reset();
// Make sure connection will not trigger again
disconnect(_connection); // FIXME Should not have to do this
}
finishedLoading(success);
}

View file

@ -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> resource, _resources) {
if (resource) {
resource->refresh();
}
}
}
@ -203,19 +203,28 @@ void ResourceCache::updateTotalSize(const qint64& oldSize, const qint64& newSize
emit dirty();
}
void ResourceCacheSharedItems::appendActiveRequest(Resource* resource) {
void ResourceCacheSharedItems::appendActiveRequest(QWeakPointer<Resource> resource) {
Lock lock(_mutex);
_loadingRequests.append(resource);
}
void ResourceCacheSharedItems::appendPendingRequest(Resource* resource) {
void ResourceCacheSharedItems::appendPendingRequest(QWeakPointer<Resource> resource) {
Lock lock(_mutex);
_pendingRequests.append(resource);
}
QList<QPointer<Resource>> ResourceCacheSharedItems::getPendingRequests() const {
Lock lock(_mutex);
return _pendingRequests;
QList<QSharedPointer<Resource>> ResourceCacheSharedItems::getPendingRequests() {
QList<QSharedPointer<Resource>> result;
{
Lock lock(_mutex);
foreach(QSharedPointer<Resource> resource, _pendingRequests) {
if (resource) {
result.append(resource);
}
}
}
return result;
}
uint32_t ResourceCacheSharedItems::getPendingRequestsCount() const {
@ -223,23 +232,33 @@ uint32_t ResourceCacheSharedItems::getPendingRequestsCount() const {
return _pendingRequests.size();
}
QList<Resource*> ResourceCacheSharedItems::getLoadingRequests() const {
Lock lock(_mutex);
return _loadingRequests;
QList<QSharedPointer<Resource>> ResourceCacheSharedItems::getLoadingRequests() {
QList<QSharedPointer<Resource>> result;
{
Lock lock(_mutex);
foreach(QSharedPointer<Resource> resource, _loadingRequests) {
if (resource) {
result.append(resource);
}
}
}
return result;
}
void ResourceCacheSharedItems::removeRequest(Resource* resource) {
void ResourceCacheSharedItems::removeRequest(QWeakPointer<Resource> resource) {
Lock lock(_mutex);
_loadingRequests.removeOne(resource);
_loadingRequests.removeAll(resource);
}
Resource* ResourceCacheSharedItems::getHighestPendingRequest() {
QSharedPointer<Resource> ResourceCacheSharedItems::getHighestPendingRequest() {
Lock lock(_mutex);
// look for the highest priority pending request
int highestIndex = -1;
float highestPriority = -FLT_MAX;
QSharedPointer<Resource> highestResource;
for (int i = 0; i < _pendingRequests.size();) {
Resource* resource = _pendingRequests.at(i).data();
auto resource = _pendingRequests.at(i).lock();
if (!resource) {
_pendingRequests.removeAt(i);
continue;
@ -248,16 +267,25 @@ Resource* ResourceCacheSharedItems::getHighestPendingRequest() {
if (priority >= highestPriority) {
highestPriority = priority;
highestIndex = i;
highestResource = resource;
}
i++;
}
if (highestIndex >= 0) {
return _pendingRequests.takeAt(highestIndex);
_pendingRequests.takeAt(highestIndex);
}
return nullptr;
return highestResource;
}
bool ResourceCache::attemptRequest(Resource* resource) {
QList<QSharedPointer<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>();
if (_requestsActive >= _requestLimit) {
@ -272,7 +300,7 @@ bool ResourceCache::attemptRequest(Resource* resource) {
return true;
}
void ResourceCache::requestCompleted(Resource* resource) {
void ResourceCache::requestCompleted(QWeakPointer<Resource> resource) {
auto sharedItems = DependencyManager::get<ResourceCacheSharedItems>();
sharedItems->removeRequest(resource);
--_requestsActive;
@ -296,11 +324,6 @@ Resource::Resource(const QUrl& url, bool delayLoad) :
_request(nullptr) {
init();
// start loading immediately unless instructed otherwise
if (!(_startedLoading || delayLoad)) {
QTimer::singleShot(0, this, &Resource::ensureLoading);
}
}
Resource::~Resource() {
@ -308,7 +331,7 @@ Resource::~Resource() {
_request->disconnect(this);
_request->deleteLater();
_request = nullptr;
ResourceCache::requestCompleted(this);
ResourceCache::requestCompleted(_self);
}
}
@ -361,7 +384,7 @@ void Resource::refresh() {
_request->disconnect(this);
_request->deleteLater();
_request = nullptr;
ResourceCache::requestCompleted(this);
ResourceCache::requestCompleted(_self);
}
init();
@ -406,7 +429,7 @@ void Resource::init() {
void Resource::attemptRequest() {
_startedLoading = true;
ResourceCache::attemptRequest(this);
ResourceCache::attemptRequest(_self);
}
void Resource::finishedLoading(bool success) {
@ -432,13 +455,16 @@ void Resource::reinsert() {
void Resource::makeRequest() {
Q_ASSERT(!_request);
if (_request) {
_request->disconnect();
_request->deleteLater();
}
_request = ResourceManager::createResourceRequest(this, _activeUrl);
if (!_request) {
qCDebug(networking).noquote() << "Failed to get request for" << _url.toDisplayString();
ResourceCache::requestCompleted(this);
ResourceCache::requestCompleted(_self);
finishedLoading(false);
return;
}
@ -470,7 +496,7 @@ void Resource::handleReplyFinished() {
return;
}
ResourceCache::requestCompleted(this);
ResourceCache::requestCompleted(_self);
auto result = _request->getResult();
if (result == ResourceRequest::Success) {

View file

@ -62,21 +62,20 @@ class ResourceCacheSharedItems : public Dependency {
using Mutex = std::mutex;
using Lock = std::unique_lock<Mutex>;
public:
void appendPendingRequest(Resource* newRequest);
void appendActiveRequest(Resource* newRequest);
void removeRequest(Resource* doneRequest);
QList<QPointer<Resource>> getPendingRequests() const;
void appendPendingRequest(QWeakPointer<Resource> newRequest);
void appendActiveRequest(QWeakPointer<Resource> newRequest);
void removeRequest(QWeakPointer<Resource> doneRequest);
QList<QSharedPointer<Resource>> getPendingRequests();
uint32_t getPendingRequestsCount() const;
QList<Resource*> getLoadingRequests() const;
Resource* getHighestPendingRequest();
QList<QSharedPointer<Resource>> getLoadingRequests();
QSharedPointer<Resource> getHighestPendingRequest();
private:
ResourceCacheSharedItems() { }
virtual ~ResourceCacheSharedItems() { }
ResourceCacheSharedItems() = default;
mutable Mutex _mutex;
QList<QPointer<Resource>> _pendingRequests;
QList<Resource*> _loadingRequests;
QList<QWeakPointer<Resource>> _pendingRequests;
QList<QWeakPointer<Resource>> _loadingRequests;
};
@ -105,13 +104,11 @@ public:
void setUnusedResourceCacheSize(qint64 unusedResourcesMaxSize);
qint64 getUnusedResourceCacheSize() const { return _unusedResourcesMaxSize; }
static const QList<Resource*> getLoadingRequests()
{ return DependencyManager::get<ResourceCacheSharedItems>()->getLoadingRequests(); }
static QList<QSharedPointer<Resource>> getLoadingRequests();
static int getPendingRequestCount()
{ return DependencyManager::get<ResourceCacheSharedItems>()->getPendingRequestsCount(); }
static int getPendingRequestCount();
ResourceCache(QObject* parent = NULL);
ResourceCache(QObject* parent = nullptr);
virtual ~ResourceCache();
void refreshAll();
@ -143,8 +140,8 @@ protected:
/// 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
static bool attemptRequest(Resource* resource);
static void requestCompleted(Resource* resource);
static bool attemptRequest(QSharedPointer<Resource> resource);
static void requestCompleted(QWeakPointer<Resource> resource);
static bool attemptHighestPriorityRequest();
private:

View file

@ -110,6 +110,9 @@ ResourceRequest* ResourceManager::createResourceRequest(QObject* parent, const Q
}
Q_ASSERT(request);
if (parent) {
QObject::connect(parent, &QObject::destroyed, request, &QObject::deleteLater);
}
request->moveToThread(&_thread);
return request;
}

View file

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

View file

@ -34,7 +34,7 @@ void ResourceTests::initTestCase() {
networkAccessManager.setCache(cache);
}
static Resource* resource = nullptr;
static QSharedPointer<Resource> resource;
static bool waitForSignal(QObject *sender, const char *signal, int timeout = 1000) {
@ -55,7 +55,8 @@ void ResourceTests::downloadFirst() {
// download the Mery fst file
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;
QEventLoop loop;
@ -67,6 +68,8 @@ void ResourceTests::downloadFirst() {
loop.connect(resource, SIGNAL(failed(QNetworkReply::NetworkError)), SLOT(quit()));
loop.connect(&timer, SIGNAL(timeout()), SLOT(quit()));
timer.start();
resource->ensureLoading();
loop.exec();
QVERIFY(resource->isLoaded());
@ -76,7 +79,8 @@ void ResourceTests::downloadAgain() {
// download the Mery fst file
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;
QEventLoop loop;
@ -88,6 +92,8 @@ void ResourceTests::downloadAgain() {
loop.connect(resource, SIGNAL(failed(QNetworkReply::NetworkError)), SLOT(quit()));
loop.connect(&timer, SIGNAL(timeout()), SLOT(quit()));
timer.start();
resource->ensureLoading();
loop.exec();
QVERIFY(resource->isLoaded());