From 886561f06ddaea86a51def1e918367430240b5ac Mon Sep 17 00:00:00 2001
From: Eric Laurent <elaurent@google.com>
Date: Thu, 28 Aug 2014 19:45:37 -0700
Subject: sound trigger: fix cross deadlock between service and client

Do not call callback methods on ISoundTriggerClient with the service mutex
locked: the client implementation will lock its mutex to process
the callback and if a request from the same client with client
mutex locked was waiting for the service mutex a deadlock occurs.

Bug: 16873049.
Change-Id: Ifc3b31fdfaed525981fa59876c041478f38b42e8
---
 services/soundtrigger/SoundTriggerHwService.cpp | 58 ++++++++++++++++---------
 1 file changed, 38 insertions(+), 20 deletions(-)

(limited to 'services/soundtrigger')

diff --git a/services/soundtrigger/SoundTriggerHwService.cpp b/services/soundtrigger/SoundTriggerHwService.cpp
index 2502e0d..b5aaee3 100644
--- a/services/soundtrigger/SoundTriggerHwService.cpp
+++ b/services/soundtrigger/SoundTriggerHwService.cpp
@@ -249,7 +249,7 @@ sp<IMemory> SoundTriggerHwService::prepareRecognitionEvent_l(
         event->data_offset = sizeof(struct sound_trigger_recognition_event);
         break;
     default:
-            return eventMemory;
+        return eventMemory;
     }
 
     size_t size = event->data_offset + event->data_size;
@@ -653,7 +653,6 @@ void SoundTriggerHwService::Module::onCallbackEvent(const sp<CallbackEvent>& eve
 {
     ALOGV("onCallbackEvent type %d", event->mType);
 
-    AutoMutex lock(mLock);
     sp<IMemory> eventMemory = event->mMemory;
 
     if (eventMemory == 0 || eventMemory->pointer() == NULL) {
@@ -668,34 +667,53 @@ void SoundTriggerHwService::Module::onCallbackEvent(const sp<CallbackEvent>& eve
     case CallbackEvent::TYPE_RECOGNITION: {
         struct sound_trigger_recognition_event *recognitionEvent =
                 (struct sound_trigger_recognition_event *)eventMemory->pointer();
+        sp<ISoundTriggerClient> client;
+        {
+            AutoMutex lock(mLock);
+            sp<Model> model = getModel(recognitionEvent->model);
+            if (model == 0) {
+                ALOGW("%s model == 0", __func__);
+                return;
+            }
+            if (model->mState != Model::STATE_ACTIVE) {
+                ALOGV("onCallbackEvent model->mState %d != Model::STATE_ACTIVE", model->mState);
+                return;
+            }
 
-        sp<Model> model = getModel(recognitionEvent->model);
-        if (model == 0) {
-            ALOGW("%s model == 0", __func__);
-            return;
+            recognitionEvent->capture_session = model->mCaptureSession;
+            model->mState = Model::STATE_IDLE;
+            client = mClient;
         }
-        if (model->mState != Model::STATE_ACTIVE) {
-            ALOGV("onCallbackEvent model->mState %d != Model::STATE_ACTIVE", model->mState);
-            return;
+        if (client != 0) {
+            client->onRecognitionEvent(eventMemory);
         }
-
-        recognitionEvent->capture_session = model->mCaptureSession;
-        mClient->onRecognitionEvent(eventMemory);
-        model->mState = Model::STATE_IDLE;
     } break;
     case CallbackEvent::TYPE_SOUNDMODEL: {
         struct sound_trigger_model_event *soundmodelEvent =
                 (struct sound_trigger_model_event *)eventMemory->pointer();
-
-        sp<Model> model = getModel(soundmodelEvent->model);
-        if (model == 0) {
-            ALOGW("%s model == 0", __func__);
-            return;
+        sp<ISoundTriggerClient> client;
+        {
+            AutoMutex lock(mLock);
+            sp<Model> model = getModel(soundmodelEvent->model);
+            if (model == 0) {
+                ALOGW("%s model == 0", __func__);
+                return;
+            }
+            client = mClient;
+        }
+        if (client != 0) {
+            client->onSoundModelEvent(eventMemory);
         }
-        mClient->onSoundModelEvent(eventMemory);
     } break;
     case CallbackEvent::TYPE_SERVICE_STATE: {
-        mClient->onServiceStateChange(eventMemory);
+        sp<ISoundTriggerClient> client;
+        {
+            AutoMutex lock(mLock);
+            client = mClient;
+        }
+        if (client != 0) {
+            client->onServiceStateChange(eventMemory);
+        }
     } break;
     default:
         LOG_ALWAYS_FATAL("onCallbackEvent unknown event type %d", event->mType);
-- 
cgit v1.1