commit 56ce2edb667408420032e4c6535b0128f4484b10
author Andy Hung <hunga@google.com> Wed Jun 12 16:03:16 2024 -0700
committer Andy Hung <hunga@google.com> Fri Jun 14 18:24:22 2024 -0700
tree 760a65ea56084a19566960d93ec27dfbce521003
parent 2718e0195adc0de3cf674a1ea98eda5e9c32e1fc

AudioFlinger: defer restartIfDisabled()

Defer restartIfDisabled() to the end of
the threadLoop worker thread, allowing execution
without any mutexes held. This prevents mutex
order issues.

To accomplish this, we add a DeferredExecutor object
to ThreadBase, which allows adding functors and dtors
to execute at the end of the worker loop,
where no mutexes are held.

Test: atest AudioPlaybackCaptureTest
Bug: 345400492
Bug: 345676143
Change-Id: I1d7f491fc1289882ce67cb7289b3bc1b58e81d23

services/audioflinger/Tracks.cpp

diff --git a/services/audioflinger/Tracks.cpp b/services/audioflinger/Tracks.cpp
index a34f6d7..ce6006c 100644
--- a/services/audioflinger/Tracks.cpp
+++ b/services/audioflinger/Tracks.cpp
@@ -314,6 +314,17 @@
     return NO_ERROR;
 }
 
+void TrackBase::deferRestartIfDisabled()
+{
+    const auto thread = mThread.promote();
+    if (thread == nullptr) return;
+    thread->getThreadloopExecutor().defer(
+            [track = wp<TrackBase>::fromExisting(this)] {
+            const auto actual = track.promote();
+            if (actual) actual->restartIfDisabled();
+        });
+}
+
 PatchTrackBase::PatchTrackBase(const sp<ClientProxy>& proxy,
         IAfThreadBase* thread, const Timeout& timeout)
     : mProxy(proxy)
@@ -2303,7 +2314,7 @@
                 waitTimeLeftMs = 0;
             }
             if (status == NOT_ENOUGH_DATA) {
-                restartIfDisabled();
+                deferRestartIfDisabled();
                 continue;
             }
         }
@@ -2315,7 +2326,7 @@
         buf.mFrameCount = outFrames;
         buf.mRaw = NULL;
         mClientProxy->releaseBuffer(&buf);
-        restartIfDisabled();
+        deferRestartIfDisabled();
         pInBuffer->frameCount -= outFrames;
         pInBuffer->raw = (int8_t *)pInBuffer->raw + outFrames * mFrameSize;
         mOutBuffer.frameCount -= outFrames;
@@ -2570,7 +2581,7 @@
     const size_t originalFrameCount = buffer->mFrameCount;
     do {
         if (status == NOT_ENOUGH_DATA) {
-            restartIfDisabled();
+            deferRestartIfDisabled();
             buffer->mFrameCount = originalFrameCount; // cleared on error, must be restored.
         }
         status = mProxy->obtainBuffer(buffer, timeOut);
@@ -2581,7 +2592,7 @@
 void PatchTrack::releaseBuffer(Proxy::Buffer* buffer)
 {
     mProxy->releaseBuffer(buffer);
-    restartIfDisabled();
+    deferRestartIfDisabled();
 
     // Check if the PatchTrack has enough data to write once in releaseBuffer().
     // If not, prevent an underrun from occurring by moving the track into FS_FILLING;