commit 59a8923027d2d6a155c0fa434ae65559318da4b6
author Eric Laurent <elaurent@google.com> Sun Jun 08 14:14:17 2014 -0700
committer Eric Laurent <elaurent@google.com> Sun Jun 08 14:16:32 2014 -0700
tree abab0761e566b204ef399689ef6e1bd5004357b4
parent 7e45ef9db7446d33c941961c14555413641e444b

audio policy service: fix potential deadlock in destructor

In the AudioCommandThread loop, release mLock before releasing
the strong reference on the service as AudioPolicyService destructor
calls AudioCommandThread::exit() which acquires mLock.

Also check exiPending in thread loop before processing pending
commands or sleeping.

Bug: 15449050.

Change-Id: I148bf21bd67ef721b5b5ee2c1a6afb185c59daa3

services/audiopolicy/AudioPolicyService.cpp

diff --git a/services/audiopolicy/AudioPolicyService.cpp b/services/audiopolicy/AudioPolicyService.cpp
index a2a0461..e86f4a2 100644
--- a/services/audiopolicy/AudioPolicyService.cpp
+++ b/services/audiopolicy/AudioPolicyService.cpp
@@ -399,7 +399,8 @@
     mLock.lock();
     while (!exitPending())
     {
-        while (!mAudioCommands.isEmpty()) {
+        sp<AudioPolicyService> svc;
+        while (!mAudioCommands.isEmpty() && !exitPending()) {
             nsecs_t curTime = systemTime();
             // commands are sorted by increasing time stamp: execute them from index 0 and up
             if (mAudioCommands[0]->mTime <= curTime) {
@@ -452,7 +453,7 @@
                     StopOutputData *data = (StopOutputData *)command->mParam.get();
                     ALOGV("AudioCommandThread() processing stop output %d",
                             data->mIO);
-                    sp<AudioPolicyService> svc = mService.promote();
+                    svc = mService.promote();
                     if (svc == 0) {
                         break;
                     }
@@ -464,7 +465,7 @@
                     ReleaseOutputData *data = (ReleaseOutputData *)command->mParam.get();
                     ALOGV("AudioCommandThread() processing release output %d",
                             data->mIO);
-                    sp<AudioPolicyService> svc = mService.promote();
+                    svc = mService.promote();
                     if (svc == 0) {
                         break;
                     }
@@ -494,7 +495,7 @@
                     } break;
                 case UPDATE_AUDIOPORT_LIST: {
                     ALOGV("AudioCommandThread() processing update audio port list");
-                    sp<AudioPolicyService> svc = mService.promote();
+                    svc = mService.promote();
                     if (svc == 0) {
                         break;
                     }
@@ -504,7 +505,7 @@
                     }break;
                 case UPDATE_AUDIOPATCH_LIST: {
                     ALOGV("AudioCommandThread() processing update audio patch list");
-                    sp<AudioPolicyService> svc = mService.promote();
+                    svc = mService.promote();
                     if (svc == 0) {
                         break;
                     }
@@ -542,9 +543,16 @@
         if (mAudioCommands.isEmpty()) {
             release_wake_lock(mName.string());
         }
-        ALOGV("AudioCommandThread() going to sleep");
-        mWaitWorkCV.waitRelative(mLock, waitTime);
-        ALOGV("AudioCommandThread() waking up");
+        // release mLock before releasing strong reference on the service as
+        // AudioPolicyService destructor calls AudioCommandThread::exit() which acquires mLock.
+        mLock.unlock();
+        svc.clear();
+        mLock.lock();
+        if (!exitPending()) {
+            ALOGV("AudioCommandThread() going to sleep");
+            mWaitWorkCV.waitRelative(mLock, waitTime);
+            ALOGV("AudioCommandThread() waking up");
+        }
     }
     mLock.unlock();
     return false;