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

diff --git a/services/soundtrigger/SoundTriggerHwService.cpp b/services/soundtrigger/SoundTriggerHwService.cpp
index 2502e0d..b5aaee3 100644 (file)
--- 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);