From 68ade4e5a5e760bb5a4b793ebfa98ca386786605 Mon Sep 17 00:00:00 2001
From: wm4 <wm4@nowhere>
Date: Tue, 2 Jun 2020 20:43:32 +0200
Subject: audio: avoid possible deadlock regression for some AOs

It's conceivable that ao->driver->reset() will make the audio API wait
for ao_read_data() (i.e. its audio callback) to return. Since we
recently moved the reset() call inside the same lock that ao_read_data()
acquires, this could deadlock. Whether this really happens depends on
how exactly the AO behaves. For example, ao_wasapi does not have this
problem. "Push" AOs are not affected either.

Fix by moving it outside of the lock. Assume ao->driver->start() will
not have this problem.

Could affect ao_sdl, ao_coreaudio (and similar rotten fruit AOs). I'm
unsure whether anyone experienced the problem in practice.
---
 audio/out/buffer.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

(limited to 'audio')

diff --git a/audio/out/buffer.c b/audio/out/buffer.c
index da0d69f0de..a274fc9f7c 100644
--- a/audio/out/buffer.c
+++ b/audio/out/buffer.c
@@ -327,6 +327,7 @@ void ao_reset(struct ao *ao)
 {
     struct buffer_state *p = ao->buffer_state;
     bool wakeup = false;
+    bool do_reset = false;
 
     pthread_mutex_lock(&p->lock);
 
@@ -334,7 +335,13 @@ void ao_reset(struct ao *ao)
         mp_ring_reset(p->buffers[n]);
 
     if (!ao->stream_silence && ao->driver->reset) {
-        ao->driver->reset(ao); // assumes the audio callback thread is stopped
+        if (ao->driver->write) {
+            ao->driver->reset(ao);
+        } else {
+            // Pull AOs may wait for ao_read_data() to return.
+            // That would deadlock if called from within the lock.
+            do_reset = true;
+        }
         p->streaming = false;
     }
     p->paused = false;
@@ -350,6 +357,9 @@ void ao_reset(struct ao *ao)
 
     pthread_mutex_unlock(&p->lock);
 
+    if (do_reset)
+        ao->driver->reset(ao);
+
     if (wakeup)
         ao_wakeup_playthread(ao);
 }
@@ -358,6 +368,7 @@ void ao_pause(struct ao *ao)
 {
     struct buffer_state *p = ao->buffer_state;
     bool wakeup = false;
+    bool do_reset = false;
 
     pthread_mutex_lock(&p->lock);
 
@@ -373,7 +384,8 @@ void ao_pause(struct ao *ao)
                     p->streaming = false;
                 }
             } else if (ao->driver->reset) {
-                ao->driver->reset(ao);
+                // See ao_reset() why this is done outside of the lock.
+                do_reset = true;
                 p->streaming = false;
             }
         }
@@ -383,6 +395,9 @@ void ao_pause(struct ao *ao)
 
     pthread_mutex_unlock(&p->lock);
 
+    if (do_reset)
+        ao->driver->reset(ao);
+
     if (wakeup)
         ao_wakeup_playthread(ao);
 }
-- 
cgit v1.2.3