commit 8b1e80bf1e9f214feea219cfe981ba533b806003
author Eric Laurent <elaurent@google.com> Tue Oct 07 09:08:47 2014 -0700
committer Eric Laurent <elaurent@google.com> Tue Oct 07 12:24:30 2014 -0700
tree 0d6b46a05d685e02750433f6779eb1ce09d84294
parent f0011d49c0f08753dc2dc254c5a70885a9986602

audio policy: fix lockup during mediaserver restart

When pre or post processing effects are applied by audio policy service
automatically there is a potential lockup if the first call into
AudioPolicyService is made while creating one of those effects.
This is because effects are created with AudioPolicyService mutex held and
effect creation calls into methods (e.g registerClient()) which also acquire
the mutex.

The fix consists in adding a new mutex to AudioPolicyEffects class and not
hold the AudioPolicyService mutex when calling methods in of class.

Bug: 17830596.
Change-Id: Ie61c3671d3147f46e99ba208f39c1a3bf180779f

services/audiopolicy/AudioPolicyInterfaceImplLegacy.cpp

diff --git a/services/audiopolicy/AudioPolicyInterfaceImplLegacy.cpp b/services/audiopolicy/AudioPolicyInterfaceImplLegacy.cpp
index 1e40bc3..9f66fd1 100644
--- a/services/audiopolicy/AudioPolicyInterfaceImplLegacy.cpp
+++ b/services/audiopolicy/AudioPolicyInterfaceImplLegacy.cpp
@@ -150,14 +150,20 @@
         return NO_INIT;
     }
     ALOGV("startOutput()");
-    Mutex::Autolock _l(mLock);
-
     // create audio processors according to stream
-    status_t status = mAudioPolicyEffects->addOutputSessionEffects(output, stream, session);
-    if (status != NO_ERROR && status != ALREADY_EXISTS) {
-        ALOGW("Failed to add effects on session %d", session);
+    sp<AudioPolicyEffects>audioPolicyEffects;
+    {
+        Mutex::Autolock _l(mLock);
+        audioPolicyEffects = mAudioPolicyEffects;
+    }
+    if (audioPolicyEffects != 0) {
+        status_t status = audioPolicyEffects->addOutputSessionEffects(output, stream, session);
+        if (status != NO_ERROR && status != ALREADY_EXISTS) {
+            ALOGW("Failed to add effects on session %d", session);
+        }
     }
 
+    Mutex::Autolock _l(mLock);
     return mpAudioPolicy->start_output(mpAudioPolicy, output, stream, session);
 }
 
@@ -178,14 +184,19 @@
                                       int session)
 {
     ALOGV("doStopOutput from tid %d", gettid());
-    Mutex::Autolock _l(mLock);
-
     // release audio processors from the stream
-    status_t status = mAudioPolicyEffects->releaseOutputSessionEffects(output, stream, session);
-    if (status != NO_ERROR && status != ALREADY_EXISTS) {
-        ALOGW("Failed to release effects on session %d", session);
+    sp<AudioPolicyEffects>audioPolicyEffects;
+    {
+        Mutex::Autolock _l(mLock);
+        audioPolicyEffects = mAudioPolicyEffects;
     }
-
+    if (audioPolicyEffects != 0) {
+        status_t status = audioPolicyEffects->releaseOutputSessionEffects(output, stream, session);
+        if (status != NO_ERROR && status != ALREADY_EXISTS) {
+            ALOGW("Failed to release effects on session %d", session);
+        }
+    }
+    Mutex::Autolock _l(mLock);
     return mpAudioPolicy->stop_output(mpAudioPolicy, output, stream, session);
 }
 
@@ -224,21 +235,26 @@
         return 0;
     }
 
-    Mutex::Autolock _l(mLock);
-    // the audio_in_acoustics_t parameter is ignored by get_input()
-    audio_io_handle_t input = mpAudioPolicy->get_input(mpAudioPolicy, inputSource, samplingRate,
-                                                   format, channelMask, (audio_in_acoustics_t) 0);
-
+    audio_io_handle_t input;
+    sp<AudioPolicyEffects>audioPolicyEffects;
+    {
+        Mutex::Autolock _l(mLock);
+        // the audio_in_acoustics_t parameter is ignored by get_input()
+        input = mpAudioPolicy->get_input(mpAudioPolicy, inputSource, samplingRate,
+                                             format, channelMask, (audio_in_acoustics_t) 0);
+        audioPolicyEffects = mAudioPolicyEffects;
+    }
     if (input == 0) {
         return input;
     }
 
-    // create audio pre processors according to input source
-    status_t status = mAudioPolicyEffects->addInputEffects(input, inputSource, audioSession);
-    if (status != NO_ERROR && status != ALREADY_EXISTS) {
-        ALOGW("Failed to add effects on input %d", input);
+    if (audioPolicyEffects != 0) {
+        // create audio pre processors according to input source
+        status_t status = audioPolicyEffects->addInputEffects(input, inputSource, audioSession);
+        if (status != NO_ERROR && status != ALREADY_EXISTS) {
+            ALOGW("Failed to add effects on input %d", input);
+        }
     }
-
     return input;
 }
 
@@ -270,13 +286,19 @@
     if (mpAudioPolicy == NULL) {
         return;
     }
-    Mutex::Autolock _l(mLock);
-    mpAudioPolicy->release_input(mpAudioPolicy, input);
 
-    // release audio processors from the input
-    status_t status = mAudioPolicyEffects->releaseInputEffects(input);
-    if(status != NO_ERROR) {
-        ALOGW("Failed to release effects on input %d", input);
+    sp<AudioPolicyEffects>audioPolicyEffects;
+    {
+        Mutex::Autolock _l(mLock);
+        mpAudioPolicy->release_input(mpAudioPolicy, input);
+        audioPolicyEffects = mAudioPolicyEffects;
+    }
+    if (audioPolicyEffects != 0) {
+        // release audio processors from the input
+        status_t status = audioPolicyEffects->releaseInputEffects(input);
+        if(status != NO_ERROR) {
+            ALOGW("Failed to release effects on input %d", input);
+        }
     }
 }
 
@@ -437,9 +459,16 @@
         *count = 0;
         return NO_INIT;
     }
-    Mutex::Autolock _l(mLock);
-
-    return mAudioPolicyEffects->queryDefaultInputEffects(audioSession, descriptors, count);
+    sp<AudioPolicyEffects>audioPolicyEffects;
+    {
+        Mutex::Autolock _l(mLock);
+        audioPolicyEffects = mAudioPolicyEffects;
+    }
+    if (audioPolicyEffects == 0) {
+        *count = 0;
+        return NO_INIT;
+    }
+    return audioPolicyEffects->queryDefaultInputEffects(audioSession, descriptors, count);
 }
 
 bool AudioPolicyService::isOffloadSupported(const audio_offload_info_t& info)