From 0ebd5f95b68a3a5c9e5509f21938c9e51e74d71b Mon Sep 17 00:00:00 2001
From: Eric Laurent <elaurent@google.com>
Date: Wed, 19 Nov 2014 19:04:52 -0800
Subject: AudioSystem: fix cross deadlock

Do not hold gLockAPS when calling
AudioPolicyService::registerClient() in get_audio_policy_service().
registerClient() will need to acquire the AudioPolicyService mutex and
if at the same time a method called from AudioPolicyService
(with mutex held) calls back into AudioSystem and get_audio_policy_service()
a cross deadlock occurs.

Same preventive fix for get_audio_flinger().

Use a separate mutex for notification client list in AudioPolicyService.
This prevents deadlocking if registerClient() is called as a consequence of
AudioFlinger calling back into AudioPolicyManager while executing a method
with AudioPolicyService locked

Bug: 18403952.
Bug: 18450065.
Change-Id: Ia832e41aede8bc6c843fc615508fbdd74e0863b5
---
 media/libmedia/AudioSystem.cpp | 101 +++++++++++++++++++++++------------------
 1 file changed, 58 insertions(+), 43 deletions(-)

(limited to 'media/libmedia/AudioSystem.cpp')

diff --git a/media/libmedia/AudioSystem.cpp b/media/libmedia/AudioSystem.cpp
index 0e608f8..1f8e9b6 100644
--- a/media/libmedia/AudioSystem.cpp
+++ b/media/libmedia/AudioSystem.cpp
@@ -51,33 +51,40 @@ size_t AudioSystem::gInBuffSize = 0;    // zero indicates cache is invalid
 sp<AudioSystem::AudioPortCallback> AudioSystem::gAudioPortCallback;
 
 // establish binder interface to AudioFlinger service
-const sp<IAudioFlinger>& AudioSystem::get_audio_flinger()
+const sp<IAudioFlinger> AudioSystem::get_audio_flinger()
 {
-    Mutex::Autolock _l(gLock);
-    if (gAudioFlinger == 0) {
-        sp<IServiceManager> sm = defaultServiceManager();
-        sp<IBinder> binder;
-        do {
-            binder = sm->getService(String16("media.audio_flinger"));
-            if (binder != 0)
-                break;
-            ALOGW("AudioFlinger not published, waiting...");
-            usleep(500000); // 0.5 s
-        } while (true);
-        if (gAudioFlingerClient == NULL) {
-            gAudioFlingerClient = new AudioFlingerClient();
-        } else {
-            if (gAudioErrorCallback) {
-                gAudioErrorCallback(NO_ERROR);
+    sp<IAudioFlinger> af;
+    sp<AudioFlingerClient> afc;
+    {
+        Mutex::Autolock _l(gLock);
+        if (gAudioFlinger == 0) {
+            sp<IServiceManager> sm = defaultServiceManager();
+            sp<IBinder> binder;
+            do {
+                binder = sm->getService(String16("media.audio_flinger"));
+                if (binder != 0)
+                    break;
+                ALOGW("AudioFlinger not published, waiting...");
+                usleep(500000); // 0.5 s
+            } while (true);
+            if (gAudioFlingerClient == NULL) {
+                gAudioFlingerClient = new AudioFlingerClient();
+            } else {
+                if (gAudioErrorCallback) {
+                    gAudioErrorCallback(NO_ERROR);
+                }
             }
+            binder->linkToDeath(gAudioFlingerClient);
+            gAudioFlinger = interface_cast<IAudioFlinger>(binder);
+            LOG_ALWAYS_FATAL_IF(gAudioFlinger == 0);
+            afc = gAudioFlingerClient;
         }
-        binder->linkToDeath(gAudioFlingerClient);
-        gAudioFlinger = interface_cast<IAudioFlinger>(binder);
-        LOG_ALWAYS_FATAL_IF(gAudioFlinger == 0);
-        gAudioFlinger->registerClient(gAudioFlingerClient);
+        af = gAudioFlinger;
     }
-
-    return gAudioFlinger;
+    if (afc != 0) {
+        af->registerClient(afc);
+    }
+    return af;
 }
 
 /* static */ status_t AudioSystem::checkAudioFlinger()
@@ -546,29 +553,37 @@ sp<AudioSystem::AudioPolicyServiceClient> AudioSystem::gAudioPolicyServiceClient
 
 
 // establish binder interface to AudioPolicy service
-const sp<IAudioPolicyService>& AudioSystem::get_audio_policy_service()
-{
-    Mutex::Autolock _l(gLockAPS);
-    if (gAudioPolicyService == 0) {
-        sp<IServiceManager> sm = defaultServiceManager();
-        sp<IBinder> binder;
-        do {
-            binder = sm->getService(String16("media.audio_policy"));
-            if (binder != 0)
-                break;
-            ALOGW("AudioPolicyService not published, waiting...");
-            usleep(500000); // 0.5 s
-        } while (true);
-        if (gAudioPolicyServiceClient == NULL) {
-            gAudioPolicyServiceClient = new AudioPolicyServiceClient();
+const sp<IAudioPolicyService> AudioSystem::get_audio_policy_service()
+{
+    sp<IAudioPolicyService> ap;
+    sp<AudioPolicyServiceClient> apc;
+    {
+        Mutex::Autolock _l(gLockAPS);
+        if (gAudioPolicyService == 0) {
+            sp<IServiceManager> sm = defaultServiceManager();
+            sp<IBinder> binder;
+            do {
+                binder = sm->getService(String16("media.audio_policy"));
+                if (binder != 0)
+                    break;
+                ALOGW("AudioPolicyService not published, waiting...");
+                usleep(500000); // 0.5 s
+            } while (true);
+            if (gAudioPolicyServiceClient == NULL) {
+                gAudioPolicyServiceClient = new AudioPolicyServiceClient();
+            }
+            binder->linkToDeath(gAudioPolicyServiceClient);
+            gAudioPolicyService = interface_cast<IAudioPolicyService>(binder);
+            LOG_ALWAYS_FATAL_IF(gAudioPolicyService == 0);
+            apc = gAudioPolicyServiceClient;
         }
-        binder->linkToDeath(gAudioPolicyServiceClient);
-        gAudioPolicyService = interface_cast<IAudioPolicyService>(binder);
-        LOG_ALWAYS_FATAL_IF(gAudioPolicyService == 0);
-        gAudioPolicyService->registerClient(gAudioPolicyServiceClient);
+        ap = gAudioPolicyService;
+    }
+    if (apc != 0) {
+        ap->registerClient(apc);
     }
 
-    return gAudioPolicyService;
+    return ap;
 }
 
 // ---------------------------------------------------------------------------
-- 
cgit v1.1