From fe9adc4850572cb04a9cf7877d0e77d0c1fde84e Mon Sep 17 00:00:00 2001
From: Eric Laurent <elaurent@google.com>
Date: Mon, 26 May 2014 16:03:08 -0700
Subject: DO NOT MERGE - audioflinger: fix deadlock upon AudioRecord creation
 error

AudioFlinger:openRecord() should not hold mClientLock when
releasing the local reference on AudioRecord as the destructor will
also lock mClientLock.
Same fix for AudioFlinger::createTrack().
Also make sure that AudioFlinger::createEffect() holds mClientLock
when clearing local reference to the Client in case of error.

Regression introduced by 021cf9634ab09c0753a40b7c9ef4ba603be5c3da

Bug: 15118096.
Change-Id: Ie961c398c8e0460bca9b95e2ee4ce6859316c275
---
 services/audioflinger/AudioFlinger.cpp | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

(limited to 'services')

diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp
index afb9d07..e174fc9 100644
--- a/services/audioflinger/AudioFlinger.cpp
+++ b/services/audioflinger/AudioFlinger.cpp
@@ -635,8 +635,12 @@ sp<IAudioTrack> AudioFlinger::createTrack(
     if (lStatus != NO_ERROR) {
         // remove local strong reference to Client before deleting the Track so that the
         // Client destructor is called by the TrackBase destructor with mClientLock held
-        Mutex::Autolock _cl(mClientLock);
-        client.clear();
+        // Don't hold mClientLock when releasing the reference on the track as the
+        // destructor will acquire it.
+        {
+            Mutex::Autolock _cl(mClientLock);
+            client.clear();
+        }
         track.clear();
         goto Exit;
     }
@@ -1173,7 +1177,7 @@ void AudioFlinger::registerClient(const sp<IAudioFlingerClient>& client)
     }
 
     // mClientLock should not be held here because ThreadBase::sendIoConfigEvent() will lock the
-    // ThreadBase mutex and teh locknig order is ThreadBase::mLock then AudioFlinger::mClientLock.
+    // ThreadBase mutex and the locking order is ThreadBase::mLock then AudioFlinger::mClientLock.
     if (clientAdded) {
         // the config change is always sent from playback or record threads to avoid deadlock
         // with AudioSystem::gLock
@@ -1419,8 +1423,12 @@ sp<IAudioRecord> AudioFlinger::openRecord(
     if (lStatus != NO_ERROR) {
         // remove local strong reference to Client before deleting the RecordTrack so that the
         // Client destructor is called by the TrackBase destructor with mClientLock held
-        Mutex::Autolock _cl(mClientLock);
-        client.clear();
+        // Don't hold mClientLock when releasing the reference on the track as the
+        // destructor will acquire it.
+        {
+            Mutex::Autolock _cl(mClientLock);
+            client.clear();
+        }
         recordTrack.clear();
         goto Exit;
     }
@@ -2380,6 +2388,11 @@ sp<IEffect> AudioFlinger::createEffect(
         if (handle != 0 && id != NULL) {
             *id = handle->id();
         }
+        if (handle == 0) {
+            // remove local strong reference to Client with mClientLock held
+            Mutex::Autolock _cl(mClientLock);
+            client.clear();
+        }
     }
 
 Exit:
-- 
cgit v1.1