From 84dc15aff62408bff63543f132282dbcc6426e51 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Wed, 23 Mar 2016 12:06:52 -0700 Subject: [PATCH] Fix potential deadlock in QML --- libraries/gl/src/gl/OffscreenQmlSurface.cpp | 34 +++++++++++++++------ libraries/shared/src/Finally.h | 4 +++ 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/libraries/gl/src/gl/OffscreenQmlSurface.cpp b/libraries/gl/src/gl/OffscreenQmlSurface.cpp index d3699b54e6..e224adad07 100644 --- a/libraries/gl/src/gl/OffscreenQmlSurface.cpp +++ b/libraries/gl/src/gl/OffscreenQmlSurface.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #include "OffscreenGLCanvas.h" #include "GLEscrow.h" @@ -84,6 +85,7 @@ protected: Queue _queue; QMutex _mutex; QWaitCondition _waitCondition; + std::atomic _rendering { false }; private: // Event-driven methods @@ -271,15 +273,25 @@ void OffscreenQmlRenderThread::resize() { } void OffscreenQmlRenderThread::render() { - if (_surface->_paused) { + // Ensure we always release the main thread + Finally releaseMainThread([this] { _waitCondition.wakeOne(); + }); + + if (_surface->_paused) { return; } - QMutexLocker locker(&_mutex); - _renderControl->sync(); - _waitCondition.wakeOne(); - locker.unlock(); + _rendering = true; + Finally unmarkRenderingFlag([this] { + _rendering = false; + }); + + { + QMutexLocker locker(&_mutex); + _renderControl->sync(); + releaseMainThread.trigger(); + } using namespace oglplus; @@ -292,6 +304,7 @@ void OffscreenQmlRenderThread::render() { _fbo->AttachTexture(Framebuffer::Target::Draw, FramebufferAttachment::Color, *texture, 0); _fbo->Complete(Framebuffer::Target::Draw); { + PROFILE_RANGE("qml_render->rendercontrol") _renderControl->render(); // FIXME The web browsers seem to be leaving GL in an error state. // Need a debug context with sync logging to figure out why. @@ -380,8 +393,6 @@ void OffscreenQmlSurface::resize(const QSize& newSize_) { std::max(static_cast(scale * newSize.height()), 10)); } - - QSize currentSize = _renderer->_quickWindow->geometry().size(); if (newSize == currentSize) { return; @@ -492,7 +503,12 @@ QObject* OffscreenQmlSurface::finishQmlLoad(std::functionallowNewFrame(_maxFps)) { + // If we're + // a) not set up + // b) already rendering a frame + // c) rendering too fast + // then skip this + if (!_renderer || _renderer->_rendering || !_renderer->allowNewFrame(_maxFps)) { return; } @@ -502,11 +518,11 @@ void OffscreenQmlSurface::updateQuick() { } if (_render) { + PROFILE_RANGE(__FUNCTION__); // Lock the GUI size while syncing QMutexLocker locker(&(_renderer->_mutex)); _renderer->_queue.add(RENDER); _renderer->_waitCondition.wait(&(_renderer->_mutex)); - _render = false; } diff --git a/libraries/shared/src/Finally.h b/libraries/shared/src/Finally.h index 59e8be0228..9070d49647 100644 --- a/libraries/shared/src/Finally.h +++ b/libraries/shared/src/Finally.h @@ -20,6 +20,10 @@ public: template Finally(F f) : _f(f) {} ~Finally() { _f(); } + void trigger() { + _f(); + _f = [] {}; + } private: std::function _f; };