From e4b1b6228db3d5a0372d5bbc2a611f5c29e0270d Mon Sep 17 00:00:00 2001 From: Robert Osfield Date: Tue, 11 Jan 2011 16:58:17 +0000 Subject: [PATCH] From Tim Moore, "This patch fixes a race condition in Renderer::ThreadSafeQueue that was causing some notifications of available SceneView objects to be missed. I saw a very noticeable performance problem (60 fps -> 8 fps) in DrawThreadPerContext mode in an osgEarth application before this patch. I had high hopes that this change might fix the much-discussed multiple GPU problem; no such luck, but I think the root cause of that is probably a similar threading issue." --- include/osgViewer/Renderer | 18 +++++++++------- src/osgViewer/Renderer.cpp | 43 +++++++++++++++++++++++++++----------- 2 files changed, 41 insertions(+), 20 deletions(-) diff --git a/include/osgViewer/Renderer b/include/osgViewer/Renderer index adb4fe25b..d423fa30a 100644 --- a/include/osgViewer/Renderer +++ b/include/osgViewer/Renderer @@ -14,6 +14,7 @@ #ifndef OSGVIEWER_RENDERER #define OSGVIEWER_RENDERER 1 +#include #include #include #include @@ -89,20 +90,21 @@ class OSGVIEWER_EXPORT Renderer : public osg::GraphicsOperation struct OSGVIEWER_EXPORT ThreadSafeQueue { OpenThreads::Mutex _mutex; - OpenThreads::Block _block; + OpenThreads::Condition _cond; typedef std::list SceneViewList; SceneViewList _queue; + bool _isReleased; ThreadSafeQueue(); ~ThreadSafeQueue(); - - void release() - { - _block.release(); - } - + + /** Release any thread waiting on the queue, even if the queue is empty. */ + void release(); + + /** Take a SceneView from the queue. Can return 0 if release() is called when the queue is empty. */ osgUtil::SceneView* takeFront(); - + + /** Add a SceneView object to the back of the queue. */ void add(osgUtil::SceneView* sv); }; diff --git a/src/osgViewer/Renderer.cpp b/src/osgViewer/Renderer.cpp index 3857b4c21..86c8e9237 100644 --- a/src/osgViewer/Renderer.cpp +++ b/src/osgViewer/Renderer.cpp @@ -289,34 +289,53 @@ void ARBQuerySupport::checkQuery(osg::Stats* stats, osg::State* state, // ThreadSafeQueue Renderer::ThreadSafeQueue::ThreadSafeQueue() + : _isReleased(false) { - _block.set(false); } Renderer::ThreadSafeQueue::~ThreadSafeQueue() { } +void Renderer::ThreadSafeQueue::release() +{ + OpenThreads::ScopedLock lock(_mutex); + _isReleased = true; + _cond.broadcast(); +} + osgUtil::SceneView* Renderer::ThreadSafeQueue::takeFront() { - if (_queue.empty()) _block.block(); - OpenThreads::ScopedLock lock(_mutex); - if (_queue.empty()) return 0; - - osgUtil::SceneView* front = _queue.front(); - _queue.pop_front(); - - if (_queue.empty()) _block.set(false); - - return front; + // Loop in case there are spurious wakeups from the condition wait. + while (true) + { + // If the queue has been released but nothing is enqueued, + // just return. This prevents a deadlock when threading is + // restarted. + if (_isReleased) + { + if (!_queue.empty()) + { + osgUtil::SceneView* front = _queue.front(); + _queue.pop_front(); + if (_queue.empty()) + _isReleased = false; + return front; + } + return 0; + } + _cond.wait(&_mutex); + } + return 0; // Can't happen } void Renderer::ThreadSafeQueue::add(osgUtil::SceneView* sv) { OpenThreads::ScopedLock lock(_mutex); _queue.push_back(sv); - _block.set(true); + _isReleased = true; + _cond.broadcast(); } static OpenThreads::Mutex s_drawSerializerMutex;