OpenGL: Avoid occasional deadlock when destroying contexts

modules/juce_opengl/opengl/juce_OpenGLContext.cpp

@@ -191,7 +191,7 @@ public:
     void pause()
     {
         signalJobShouldExit();
-        contextsWaitingForFlush->cancelFlush (this);
+        contextsWaitingForFlush->deregisterContext (this);
         messageManagerLock.abort();
 
         if (renderThread != nullptr)
@@ -345,8 +345,7 @@ public:
             OpenGLContext::deactivateCurrentContext();
         }
 
-        if (! shouldExit())
-            contextsWaitingForFlush->flush (this);
+        contextsWaitingForFlush->flush (this);
 
         return true;
     }
@@ -551,6 +550,8 @@ public:
             return ThreadPoolJob::jobHasFinished;
         }
 
+        contextsWaitingForFlush->registerContext (this);
+
         hasInitialised = true;
 
         while (! shouldExit())
@@ -777,6 +778,19 @@ public:
     class ContextsWaitingForFlush : private AsyncUpdater
     {
     public:
+        void registerContext (CachedImage* ctx)
+        {
+            const std::lock_guard<std::mutex> lock (mutex);
+            needsRender.insert_or_assign (ctx, false);
+        }
+
+        void deregisterContext (CachedImage* ctx)
+        {
+            const std::lock_guard<std::mutex> lock (mutex);
+            needsRender.erase (ctx);
+            condvar.notify_all();
+        }
+
         /*  Ask to swap the CachedImage's buffers on the main thread.
 
             Will block until the buffers have been swapped, or until the swap has been cancelled.
@@ -786,8 +800,8 @@ public:
             {
                 const std::lock_guard<std::mutex> lock (mutex);
 
-                if (find (ctx) == contexts.cend())
-                    contexts.push_back (ctx);
+                if (const auto iter = needsRender.find (ctx); iter != needsRender.cend())
+                    iter->second = true;
             }
 
             if (MessageManager::getInstance()->isThisTheMessageThread())
@@ -799,21 +813,15 @@ public:
                 triggerAsyncUpdate();
 
                 std::unique_lock<std::mutex> lock { mutex };
-                condvar.wait (lock, [this, ctx] { return find (ctx) == contexts.cend(); });
-            }
-        }
-
-        /*  When a context is destroyed, this function must be called so that flush() can return. */
-        void cancelFlush (CachedImage* ctx)
-        {
-            const std::lock_guard<std::mutex> lock (mutex);
-
-            const auto iter = find (ctx);
+                condvar.wait (lock, [this, ctx]
+                {
+                    // If the context is still registered, continue once it no longer needs rendering.
+                    if (const auto iter = needsRender.find (ctx); iter != needsRender.cend())
+                        return ! iter->second;
 
-            if (iter != contexts.cend())
-            {
-                contexts.erase (iter);
-                condvar.notify_all();
+                    // If the context is no longer registered, continue so that the thread can be destroyed.
+                    return true;
+                });
             }
         }
 
@@ -822,40 +830,35 @@ public:
             cancelPendingUpdate();
 
             // There definitely shouldn't still be active CachedImages if this object is being destroyed!
-            jassert (contexts.empty());
+            jassert (needsRender.empty());
         }
 
     private:
         std::mutex mutex;
         std::condition_variable condvar;
-        std::vector<CachedImage*> contexts;
-
-        /*  Precondition: mutex is locked. */
-        std::vector<CachedImage*>::const_iterator find (CachedImage* ctx) const
-        {
-            // Linear search here because the number of OpenGL contexts probably won't
-            // ever be bigger than double/triple digits
-            return std::find (contexts.cbegin(), contexts.cend(), ctx);
-        }
+        std::map<CachedImage*, bool> needsRender;
 
         /*  Swaps the buffers for each of the pending contexts, then notifies all rendering threads
             that they may continue.
         */
         void handleAsyncUpdate() override
         {
-            std::unique_lock<std::mutex> lock (mutex);
+            const std::unique_lock<std::mutex> lock (mutex);
 
-            for (auto* ctx : contexts)
+            for (auto& [ctx, shouldSwap] : needsRender)
             {
+                if (! shouldSwap)
+                    continue;
+
                 auto& native = *ctx->nativeContext;
 
-                OpenGLContext::NativeContext::Locker nativeContextLocker (native);
+                const OpenGLContext::NativeContext::Locker nativeContextLocker { native };
                 native.makeActive();
                 native.swapBuffers();
                 native.deactivateCurrentContext();
+                shouldSwap = false;
             }
 
-            contexts.clear();
             condvar.notify_all();
         }
     };