From 6d525b3f0a75c47ef2f5b376e7b909568c3205fd Mon Sep 17 00:00:00 2001
From: James H Ball <james@ball.sh>
Date: Sat, 26 Apr 2025 21:40:15 +0100
Subject: [PATCH] Add timeout to semaphore acquires, as well as several
 pre-emptive semaphore releases to try and reduce the scope of deadlocks. i.e.
 we prefer race conditions to outright crashes/deadlocks

---
 Source/concurrency/AudioBackgroundThread.cpp |  7 ++++---
 Source/concurrency/BufferConsumer.h          | 17 +++++++++++------
 Source/visualiser/VisualiserComponent.cpp    | 19 ++++++++++++++++++-
 3 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/Source/concurrency/AudioBackgroundThread.cpp b/Source/concurrency/AudioBackgroundThread.cpp
index d82ea11..db2ca57 100644
--- a/Source/concurrency/AudioBackgroundThread.cpp
+++ b/Source/concurrency/AudioBackgroundThread.cpp
@@ -51,9 +51,10 @@ void AudioBackgroundThread::write(const OsciPoint& point) {
 
 void AudioBackgroundThread::run() {
     while (!threadShouldExit() && shouldBeRunning) {
-        consumer->waitUntilFull();
-        if (shouldBeRunning) {
-            runTask(consumer->getBuffer());
+        if (consumer->waitUntilFull()) {
+            if (shouldBeRunning) {
+                runTask(consumer->getBuffer());
+            }
         }
     }
 }
diff --git a/Source/concurrency/BufferConsumer.h b/Source/concurrency/BufferConsumer.h
index b90a4a7..fc1a693 100644
--- a/Source/concurrency/BufferConsumer.h
+++ b/Source/concurrency/BufferConsumer.h
@@ -21,11 +21,14 @@ public:
         only the number of permissions and number of available permissions **/
     Semaphore(const Semaphore& s) : num_permissions(s.num_permissions), avail(s.avail) { }
 
-    void acquire() {
+    bool acquire(std::chrono::milliseconds timeout = std::chrono::milliseconds(200)) {
         std::unique_lock<std::mutex> lk(m);
-        cv.wait(lk, [this] { return avail > 0; });
-        avail--;
+        bool result = cv.wait_for(lk, timeout, [this] { return avail > 0; });
+        if (result) {
+            avail--;
+        }
         lk.unlock();
+        return result;
     }
 
     void release() {
@@ -61,13 +64,15 @@ public:
     // PRODUCER
     // enqueue point
     
-    void waitUntilFull() {
+    bool waitUntilFull() {
         if (blockOnWrite) {
+            bool writtenSuccessfully = true;
             for (int i = 0; i < returnBuffer.size() && blockOnWrite; i++) {
-                queue->wait_dequeue(returnBuffer[i]);
+                writtenSuccessfully = writtenSuccessfully && queue->wait_dequeue_timed(returnBuffer[i], 1000000);
             }
+            return writtenSuccessfully;
         } else {
-            sema.acquire();
+            return sema.acquire();
         }
     }
     
diff --git a/Source/visualiser/VisualiserComponent.cpp b/Source/visualiser/VisualiserComponent.cpp
index 7168789..8ad40c2 100644
--- a/Source/visualiser/VisualiserComponent.cpp
+++ b/Source/visualiser/VisualiserComponent.cpp
@@ -156,6 +156,10 @@ void VisualiserComponent::setFullScreen(bool fullScreen) {
     this->fullScreen = fullScreen;
     hideButtonRow = false;
     setMouseCursor(juce::MouseCursor::PointingHandCursor);
+    
+    // Release renderingSemaphore to prevent deadlocks during layout changes
+    renderingSemaphore.release();
+    
     resized();
 }
 
@@ -290,7 +294,10 @@ void VisualiserComponent::runTask(const std::vector<OsciPoint>& points) {
     // this just triggers a repaint
     triggerAsyncUpdate();
     // wait for rendering on the OpenGLRenderer thread to complete
-    renderingSemaphore.acquire();
+    if (!renderingSemaphore.acquire()) {
+        // If acquire times out, log a message or handle it as appropriate
+        juce::Logger::writeToLog("Rendering semaphore acquisition timed out");
+    }
 }
 
 int VisualiserComponent::prepareTask(double sampleRate, int bufferSize) {
@@ -397,6 +404,9 @@ void VisualiserComponent::setRecording(bool recording) {
     bool stillRecording = audioRecorder.isRecording();
 #endif
 
+    // Release renderingSemaphore to prevent deadlock
+    renderingSemaphore.release();
+
     if (recording) {
 #if SOSCI_FEATURES
         recordingVideo = recordingSettings.recordingVideo();
@@ -609,6 +619,10 @@ void VisualiserComponent::popoutWindow() {
     }
 #endif
     setRecording(false);
+    
+    // Release renderingSemaphore to prevent deadlock when creating a child visualizer
+    renderingSemaphore.release();
+    
     auto visualiser = new VisualiserComponent(
         audioProcessor,
 #if SOSCI_FEATURES
@@ -957,6 +971,9 @@ Texture VisualiserComponent::makeTexture(int width, int height, GLuint textureID
 void VisualiserComponent::setResolution(int width) {
     using namespace juce::gl;
 
+    // Release semaphore to prevent deadlocks during texture rebuilding
+    renderingSemaphore.release();
+
     glBindFramebuffer(GL_FRAMEBUFFER, frameBuffer);
 
     lineTexture = makeTexture(width, width, lineTexture.id);