fix deadlock in audioflinger::createEffect()

commit 5baf2af5 introduced a regression by calling
getOutputForEffect() with AudioFLinger main mutex locked.
The locking order must always be AudioPolicyService mutex then
AudioFlinger then ThreadBase mutex.

Bug: 10916796.
Change-Id: Ide34a2d84dbb06dbb35abd0640d91b01b0ac4d40

2 files changed, 22 insertions, 21 deletions

diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp
index f6e4c6a..e70d566 100644
--- a/services/audioflinger/AudioFlinger.cpp
+++ b/services/audioflinger/AudioFlinger.cpp
@@ -2102,9 +2102,6 @@ sp<IEffect> AudioFlinger::createEffect(
     }
 
     {
-        Mutex::Autolock _l(mLock);
-
-
         if (!EffectIsNullUuid(&pDesc->uuid)) {
             // if uuid is specified, request effect descriptor
             lStatus = EffectGetDescriptor(&pDesc->uuid, &desc);
@@ -2177,6 +2174,15 @@ sp<IEffect> AudioFlinger::createEffect(
 
         // return effect descriptor
         *pDesc = desc;
+        if (io == 0 && sessionId == AUDIO_SESSION_OUTPUT_MIX) {
+            // if the output returned by getOutputForEffect() is removed before we lock the
+            // mutex below, the call to checkPlaybackThread_l(io) below will detect it
+            // and we will exit safely
+            io = AudioSystem::getOutputForEffect(&desc);
+            ALOGV("createEffect got output %d", io);
+        }
+
+        Mutex::Autolock _l(mLock);
 
         // If output is not specified try to find a matching audio session ID in one of the
         // output threads.
@@ -2190,29 +2196,20 @@ sp<IEffect> AudioFlinger::createEffect(
                 lStatus = BAD_VALUE;
                 goto Exit;
             }
-            if (sessionId == AUDIO_SESSION_OUTPUT_MIX) {
-                // if the output returned by getOutputForEffect() is removed before we lock the
-                // mutex below, the call to checkPlaybackThread_l(io) below will detect it
-                // and we will exit safely
-                io = AudioSystem::getOutputForEffect(&desc);
-                ALOGV("createEffect got output %d", io);
+            // look for the thread where the specified audio session is present
+            for (size_t i = 0; i < mPlaybackThreads.size(); i++) {
+                if (mPlaybackThreads.valueAt(i)->hasAudioSession(sessionId) != 0) {
+                    io = mPlaybackThreads.keyAt(i);
+                    break;
+                }
             }
             if (io == 0) {
-                // look for the thread where the specified audio session is present
-                for (size_t i = 0; i < mPlaybackThreads.size(); i++) {
-                    if (mPlaybackThreads.valueAt(i)->hasAudioSession(sessionId) != 0) {
-                        io = mPlaybackThreads.keyAt(i);
+                for (size_t i = 0; i < mRecordThreads.size(); i++) {
+                    if (mRecordThreads.valueAt(i)->hasAudioSession(sessionId) != 0) {
+                        io = mRecordThreads.keyAt(i);
                         break;
                     }
                 }
-                if (io == 0) {
-                    for (size_t i = 0; i < mRecordThreads.size(); i++) {
-                        if (mRecordThreads.valueAt(i)->hasAudioSession(sessionId) != 0) {
-                            io = mRecordThreads.keyAt(i);
-                            break;
-                        }
-                    }
-                }
             }
             // If no output thread contains the requested session ID, default to
             // first output. The effect chain will be moved to the correct output
diff --git a/services/audioflinger/Effects.h b/services/audioflinger/Effects.h
index c35cff0..b717857 100644
--- a/services/audioflinger/Effects.h
+++ b/services/audioflinger/Effects.h
@@ -25,6 +25,10 @@
 // state changes or resource modifications. Always respect the following order
 // if multiple mutexes must be acquired to avoid cross deadlock:
 // AudioFlinger -> ThreadBase -> EffectChain -> EffectModule
+// In addition, methods that lock the AudioPolicyService mutex (getOutputForEffect(),
+// startOutput()...) should never be called with AudioFlinger or Threadbase mutex locked
+// to avoid cross deadlock with other clients calling AudioPolicyService methods that in turn
+// call AudioFlinger thus locking the same mutexes in the reverse order.
 
 // The EffectModule class is a wrapper object controlling the effect engine implementation
 // in the effect library. It prevents concurrent calls to process() and command() functions