Avoid deadlock between recognition events and SoundTriggerHwService.
If an event is generated from the HAL on a separate thread, it is
possible to enter a deadlocked state. This is due to the HAL typically
holding a lock when it starts the callback, which then needed to grab
the SoundTriggerHwService's main lock. However, if there was another
call coming down from somewhere else in the framework, it typically
tries to grab the service's main lock, then enter the HAL (which also
will likely try to grab a lock).
By removing the need to grab the mServiceLock on any of the paths that
respond to events generated by the HAL, this deadlock can be avoided.
Test: Manually on device.
Change-Id: Iac5f96ea67f618ce70e7f75300165f3588613947
diff --git a/services/soundtrigger/SoundTriggerHwService.cpp b/services/soundtrigger/SoundTriggerHwService.cpp
index 22519a3..46f388b 100644
--- a/services/soundtrigger/SoundTriggerHwService.cpp
+++ b/services/soundtrigger/SoundTriggerHwService.cpp
@@ -206,9 +206,10 @@
service->sendRecognitionEvent(event, module);
}
-sp<IMemory> SoundTriggerHwService::prepareRecognitionEvent_l(
+sp<IMemory> SoundTriggerHwService::prepareRecognitionEvent(
struct sound_trigger_recognition_event *event)
{
+ AutoMutex lock(mMemoryDealerLock);
sp<IMemory> eventMemory;
//sanitize event
@@ -216,21 +217,21 @@
case SOUND_MODEL_TYPE_KEYPHRASE:
ALOGW_IF(event->data_size != 0 && event->data_offset !=
sizeof(struct sound_trigger_phrase_recognition_event),
- "prepareRecognitionEvent_l(): invalid data offset %u for keyphrase event type",
+ "prepareRecognitionEvent(): invalid data offset %u for keyphrase event type",
event->data_offset);
event->data_offset = sizeof(struct sound_trigger_phrase_recognition_event);
break;
case SOUND_MODEL_TYPE_GENERIC:
ALOGW_IF(event->data_size != 0 && event->data_offset !=
sizeof(struct sound_trigger_generic_recognition_event),
- "prepareRecognitionEvent_l(): invalid data offset %u for generic event type",
+ "prepareRecognitionEvent(): invalid data offset %u for generic event type",
event->data_offset);
event->data_offset = sizeof(struct sound_trigger_generic_recognition_event);
break;
case SOUND_MODEL_TYPE_UNKNOWN:
ALOGW_IF(event->data_size != 0 && event->data_offset !=
sizeof(struct sound_trigger_recognition_event),
- "prepareRecognitionEvent_l(): invalid data offset %u for unknown event type",
+ "prepareRecognitionEvent(): invalid data offset %u for unknown event type",
event->data_offset);
event->data_offset = sizeof(struct sound_trigger_recognition_event);
break;
@@ -251,30 +252,19 @@
void SoundTriggerHwService::sendRecognitionEvent(struct sound_trigger_recognition_event *event,
Module *module)
- {
- AutoMutex lock(mServiceLock);
- if (module == NULL) {
- return;
- }
- sp<IMemory> eventMemory = prepareRecognitionEvent_l(event);
- if (eventMemory == 0) {
- return;
- }
- sp<Module> strongModule;
- for (size_t i = 0; i < mModules.size(); i++) {
- if (mModules.valueAt(i).get() == module) {
- strongModule = mModules.valueAt(i);
- break;
- }
- }
- if (strongModule == 0) {
- return;
- }
+{
+ if (module == NULL) {
+ return;
+ }
+ sp<IMemory> eventMemory = prepareRecognitionEvent(event);
+ if (eventMemory == 0) {
+ return;
+ }
sp<CallbackEvent> callbackEvent = new CallbackEvent(CallbackEvent::TYPE_RECOGNITION,
eventMemory);
- callbackEvent->setModule(strongModule);
- sendCallbackEvent_l(callbackEvent);
+ callbackEvent->setModule(module);
+ sendCallbackEvent(callbackEvent);
}
// static
@@ -293,8 +283,9 @@
service->sendSoundModelEvent(event, module);
}
-sp<IMemory> SoundTriggerHwService::prepareSoundModelEvent_l(struct sound_trigger_model_event *event)
+sp<IMemory> SoundTriggerHwService::prepareSoundModelEvent(struct sound_trigger_model_event *event)
{
+ AutoMutex lock(mMemoryDealerLock);
sp<IMemory> eventMemory;
size_t size = event->data_offset + event->data_size;
@@ -311,30 +302,20 @@
void SoundTriggerHwService::sendSoundModelEvent(struct sound_trigger_model_event *event,
Module *module)
{
- AutoMutex lock(mServiceLock);
- sp<IMemory> eventMemory = prepareSoundModelEvent_l(event);
+ sp<IMemory> eventMemory = prepareSoundModelEvent(event);
if (eventMemory == 0) {
return;
}
- sp<Module> strongModule;
- for (size_t i = 0; i < mModules.size(); i++) {
- if (mModules.valueAt(i).get() == module) {
- strongModule = mModules.valueAt(i);
- break;
- }
- }
- if (strongModule == 0) {
- return;
- }
sp<CallbackEvent> callbackEvent = new CallbackEvent(CallbackEvent::TYPE_SOUNDMODEL,
eventMemory);
- callbackEvent->setModule(strongModule);
- sendCallbackEvent_l(callbackEvent);
+ callbackEvent->setModule(module);
+ sendCallbackEvent(callbackEvent);
}
sp<IMemory> SoundTriggerHwService::prepareServiceStateEvent_l(sound_trigger_service_state_t state)
{
+ AutoMutex lock(mMemoryDealerLock);
sp<IMemory> eventMemory;
size_t size = sizeof(sound_trigger_service_state_t);
@@ -368,7 +349,7 @@
sp<CallbackEvent> callbackEvent = new CallbackEvent(CallbackEvent::TYPE_SERVICE_STATE,
eventMemory);
callbackEvent->setModule(strongModule);
- sendCallbackEvent_l(callbackEvent);
+ sendCallbackEvent(callbackEvent);
}
void SoundTriggerHwService::sendServiceStateEvent_l(sound_trigger_service_state_t state,
@@ -381,11 +362,10 @@
sp<CallbackEvent> callbackEvent = new CallbackEvent(CallbackEvent::TYPE_SERVICE_STATE,
eventMemory);
callbackEvent->setModuleClient(moduleClient);
- sendCallbackEvent_l(callbackEvent);
+ sendCallbackEvent(callbackEvent);
}
-// call with mServiceLock held
-void SoundTriggerHwService::sendCallbackEvent_l(const sp<CallbackEvent>& event)
+void SoundTriggerHwService::sendCallbackEvent(const sp<CallbackEvent>& event)
{
mCallbackThread->sendCallbackEvent(event);
}
@@ -404,6 +384,19 @@
if (moduleClient == 0) {
return;
}
+ } else {
+ // Sanity check on this being a Module we know about.
+ bool foundModule = false;
+ for (size_t i = 0; i < mModules.size(); i++) {
+ if (mModules.valueAt(i).get() == module.get()) {
+ foundModule = true;
+ break;
+ }
+ }
+ if (!foundModule) {
+ ALOGE("onCallbackEvent for unknown module");
+ return;
+ }
}
}
if (module != 0) {
@@ -878,7 +871,7 @@
event.common.type = model->mType;
event.common.model = model->mHandle;
event.common.data_size = 0;
- sp<IMemory> eventMemory = service->prepareRecognitionEvent_l(&event.common);
+ sp<IMemory> eventMemory = service->prepareRecognitionEvent(&event.common);
if (eventMemory != 0) {
events.add(eventMemory);
}
@@ -889,7 +882,7 @@
event.common.type = model->mType;
event.common.model = model->mHandle;
event.common.data_size = 0;
- sp<IMemory> eventMemory = service->prepareRecognitionEvent_l(&event.common);
+ sp<IMemory> eventMemory = service->prepareRecognitionEvent(&event.common);
if (eventMemory != 0) {
events.add(eventMemory);
}
@@ -900,7 +893,7 @@
event.common.type = model->mType;
event.common.model = model->mHandle;
event.common.data_size = 0;
- sp<IMemory> eventMemory = service->prepareRecognitionEvent_l(&event.common);
+ sp<IMemory> eventMemory = service->prepareRecognitionEvent(&event.common);
if (eventMemory != 0) {
events.add(eventMemory);
}
@@ -915,7 +908,7 @@
sp<CallbackEvent> callbackEvent = new CallbackEvent(CallbackEvent::TYPE_RECOGNITION,
events[i]);
callbackEvent->setModule(this);
- service->sendCallbackEvent_l(callbackEvent);
+ service->sendCallbackEvent(callbackEvent);
}
exit:
diff --git a/services/soundtrigger/SoundTriggerHwService.h b/services/soundtrigger/SoundTriggerHwService.h
index 95efc4b..a955f40 100644
--- a/services/soundtrigger/SoundTriggerHwService.h
+++ b/services/soundtrigger/SoundTriggerHwService.h
@@ -214,11 +214,11 @@
};
static void recognitionCallback(struct sound_trigger_recognition_event *event, void *cookie);
- sp<IMemory> prepareRecognitionEvent_l(struct sound_trigger_recognition_event *event);
+ sp<IMemory> prepareRecognitionEvent(struct sound_trigger_recognition_event *event);
void sendRecognitionEvent(struct sound_trigger_recognition_event *event, Module *module);
static void soundModelCallback(struct sound_trigger_model_event *event, void *cookie);
- sp<IMemory> prepareSoundModelEvent_l(struct sound_trigger_model_event *event);
+ sp<IMemory> prepareSoundModelEvent(struct sound_trigger_model_event *event);
void sendSoundModelEvent(struct sound_trigger_model_event *event, Module *module);
sp<IMemory> prepareServiceStateEvent_l(sound_trigger_service_state_t state);
@@ -226,7 +226,7 @@
void sendServiceStateEvent_l(sound_trigger_service_state_t state,
ModuleClient *moduleClient);
- void sendCallbackEvent_l(const sp<CallbackEvent>& event);
+ void sendCallbackEvent(const sp<CallbackEvent>& event);
void onCallbackEvent(const sp<CallbackEvent>& event);
private:
@@ -238,6 +238,7 @@
DefaultKeyedVector< sound_trigger_module_handle_t, sp<Module> > mModules;
sp<CallbackThread> mCallbackThread;
sp<MemoryDealer> mMemoryDealer;
+ Mutex mMemoryDealerLock;
bool mCaptureState;
};