From 069c358aa3f6447a22520f9967d3ff3bdcf7786c Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Tue, 17 Mar 2015 11:38:42 -0700 Subject: [PATCH] Work around a deadlock: thread 15 locks Octree thread 1 blocks waiting for Octree lock thread 15 tries to pass a message to thread 1 with QMetaObject::invokeMethod, and hangs thread 15 is blocked on qt message passing to thread 1 ResourceCache::getResource libraries/networking/src/ResourceCache.cpp:57 GeometryCache::getGeometry libraries/render-utils/src/GeometryCache.cpp:1774 ModelEntityItem::isReadyToComputeShape libraries/entities/src/ModelEntityItem.cpp:431 PhysicsEngine::addEntityInternal libraries/physics/src/PhysicsEngine.cpp:67 EntitySimulation::addEntity libraries/entities/src/EntitySimulation.cpp:129 EntityTree::postAddEntity libraries/entities/src/EntityTree.cpp:91 ^ locks simulation EntityTreeElement::readElementDataFromBuffer libraries/entities/src/EntityTreeElement.cpp:773 Octree::readElementData libraries/octree/src/Octree.cpp:301 Octree::readElementData libraries/octree/src/Octree.cpp:354 ... Octree::readBitstreamToTree libraries/octree/src/Octree.cpp:439 OctreeRenderer::processDatagram libraries/octree/src/OctreeRenderer.cpp:136 ^ lockForWrite Octree::_lock OctreePacketProcessor::processPacket interface/src/octree/OctreePacketProcessor.cpp:91 ReceivedPacketProcessor::process libraries/networking/src/ReceivedPacketProcessor.cpp:51 thread 1 is blocked on lockForWrite of Octree::_lock Octree::lockForWrite libraries/octree/src/Octree.h:292 EntityTree::update libraries/entities/src/EntityTree.cpp:668 ^ lockForWrite on Octree:_lock EntityTreeRenderer::update libraries/entities-renderer/src/EntityTreeRenderer.cpp:258 Application::update interface/src/Application.cpp:2189 ^ calls _physicsEngine.stepSimulation() before this Application::idle interface/src/Application.cpp:1535 timer --- libraries/entities/src/ModelEntityItem.cpp | 9 ++++--- libraries/networking/src/ResourceCache.cpp | 26 ++++++++++++++------ libraries/networking/src/ResourceCache.h | 2 +- libraries/render-utils/src/GeometryCache.cpp | 4 +-- libraries/render-utils/src/GeometryCache.h | 3 ++- 5 files changed, 30 insertions(+), 14 deletions(-) diff --git a/libraries/entities/src/ModelEntityItem.cpp b/libraries/entities/src/ModelEntityItem.cpp index 1f72c2e93d..4e8223cfab 100644 --- a/libraries/entities/src/ModelEntityItem.cpp +++ b/libraries/entities/src/ModelEntityItem.cpp @@ -419,19 +419,22 @@ QString ModelEntityItem::getAnimationSettings() const { bool ModelEntityItem::isReadyToComputeShape() { if (_collisionModelURL == "") { + // no model url, so we're ready to compute a shape. return true; } if (! _collisionNetworkGeometry.isNull() && _collisionNetworkGeometry->isLoadedWithTextures()) { + // we have a _collisionModelURL AND a _collisionNetworkGeometry AND it's fully loaded. return true; } if (_collisionNetworkGeometry.isNull()) { - + // we have a _collisionModelURL but we don't yet have a _collisionNetworkGeometry. _collisionNetworkGeometry = - DependencyManager::get()->getGeometry(_collisionModelURL, QUrl(), false); + DependencyManager::get()->getGeometry(_collisionModelURL, QUrl(), false, false); - if (_collisionNetworkGeometry->isLoadedWithTextures()) { + if (! _collisionNetworkGeometry.isNull() && _collisionNetworkGeometry->isLoadedWithTextures()) { + // shortcut in case it's already loaded. return true; } } diff --git a/libraries/networking/src/ResourceCache.cpp b/libraries/networking/src/ResourceCache.cpp index dfb0d6c150..b574eb1aeb 100644 --- a/libraries/networking/src/ResourceCache.cpp +++ b/libraries/networking/src/ResourceCache.cpp @@ -48,14 +48,26 @@ void ResourceCache::refresh(const QUrl& url) { } } -QSharedPointer ResourceCache::getResource(const QUrl& url, const QUrl& fallback, bool delayLoad, void* extra) { - +QSharedPointer ResourceCache::getResource(const QUrl& url, const QUrl& fallback, + bool delayLoad, void* extra, bool block) { if (QThread::currentThread() != thread()) { - QSharedPointer result; - QMetaObject::invokeMethod(this, "getResource", Qt::BlockingQueuedConnection, - Q_RETURN_ARG(QSharedPointer, result), Q_ARG(const QUrl&, url), Q_ARG(const QUrl&, fallback), - Q_ARG(bool, delayLoad), Q_ARG(void*, extra)); - return result; + // This will re-call this method in the main thread. If block is true and the main thread + // is waiting on a lock, we'll deadlock here. + if (block) { + QSharedPointer result; + QMetaObject::invokeMethod(this, "getResource", Qt::BlockingQueuedConnection, + Q_RETURN_ARG(QSharedPointer, result), Q_ARG(const QUrl&, url), + Q_ARG(const QUrl&, fallback), Q_ARG(bool, delayLoad), Q_ARG(void*, extra)); + return result; + } else { + // Queue the re-invocation of this method, but if the main thread is blocked, don't wait. The + // return value may be NULL -- it's expected that this will be called again later, in order + // to receive the actual Resource. + QMetaObject::invokeMethod(this, "getResource", Qt::QueuedConnection, + Q_ARG(const QUrl&, url), + Q_ARG(const QUrl&, fallback), Q_ARG(bool, delayLoad), Q_ARG(void*, extra)); + return _resources.value(url); + } } if (!url.isValid() && !url.isEmpty() && fallback.isValid()) { diff --git a/libraries/networking/src/ResourceCache.h b/libraries/networking/src/ResourceCache.h index 11b091a9e3..d4aa9a7fd9 100644 --- a/libraries/networking/src/ResourceCache.h +++ b/libraries/networking/src/ResourceCache.h @@ -89,7 +89,7 @@ protected: /// \param delayLoad if true, don't load the resource immediately; wait until load is first requested /// \param extra extra data to pass to the creator, if appropriate Q_INVOKABLE QSharedPointer getResource(const QUrl& url, const QUrl& fallback = QUrl(), - bool delayLoad = false, void* extra = NULL); + bool delayLoad = false, void* extra = NULL, bool block = true); /// Creates a new resource. virtual QSharedPointer createResource(const QUrl& url, diff --git a/libraries/render-utils/src/GeometryCache.cpp b/libraries/render-utils/src/GeometryCache.cpp index b80cc04b3f..29f76291ea 100644 --- a/libraries/render-utils/src/GeometryCache.cpp +++ b/libraries/render-utils/src/GeometryCache.cpp @@ -1770,8 +1770,8 @@ void GeometryCache::renderLine(const glm::vec2& p1, const glm::vec2& p2, } -QSharedPointer GeometryCache::getGeometry(const QUrl& url, const QUrl& fallback, bool delayLoad) { - return getResource(url, fallback, delayLoad).staticCast(); +QSharedPointer GeometryCache::getGeometry(const QUrl& url, const QUrl& fallback, bool delayLoad, bool block) { + return getResource(url, fallback, delayLoad, NULL, block).staticCast(); } QSharedPointer GeometryCache::createResource(const QUrl& url, diff --git a/libraries/render-utils/src/GeometryCache.h b/libraries/render-utils/src/GeometryCache.h index 37156a6c71..a64d041fc1 100644 --- a/libraries/render-utils/src/GeometryCache.h +++ b/libraries/render-utils/src/GeometryCache.h @@ -203,7 +203,8 @@ public: /// Loads geometry from the specified URL. /// \param fallback a fallback URL to load if the desired one is unavailable /// \param delayLoad if true, don't load the geometry immediately; wait until load is first requested - QSharedPointer getGeometry(const QUrl& url, const QUrl& fallback = QUrl(), bool delayLoad = false); + QSharedPointer getGeometry(const QUrl& url, const QUrl& fallback = QUrl(), + bool delayLoad = false, bool block = true); protected: