Fix race when handling callback in SoundTrigger Module
During the callback event processing loop in the SoundTrigger::Module,
it used to grab the ISoundTriggerClient from the ModuleClient that it
needed to send an event to. However, this could be removed/destroyed
while the callback thread is trying to snapshot it, since there was no
common locking between the model client and the module.
By now using the ModuleClient directly in the Module's callback, the
appropriate locks can be obtained to prevent segfaults.
Test: On device with GSA/Now Playing
Change-Id: I9e3d150aa77051caab1fc009c3fc1ae8eec61a72
diff --git a/services/soundtrigger/SoundTriggerHwService.cpp b/services/soundtrigger/SoundTriggerHwService.cpp
index 46f388b..6d833aa 100644
--- a/services/soundtrigger/SoundTriggerHwService.cpp
+++ b/services/soundtrigger/SoundTriggerHwService.cpp
@@ -750,11 +750,12 @@
return;
}
+ Vector< sp<ModuleClient> > clients;
+
switch (event->mType) {
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);
@@ -769,16 +770,12 @@
recognitionEvent->capture_session = model->mCaptureSession;
model->mState = Model::STATE_IDLE;
- client = model->mModuleClient->client();
- }
- if (client != 0) {
- client->onRecognitionEvent(eventMemory);
+ clients.add(model->mModuleClient);
}
} break;
case CallbackEvent::TYPE_SOUNDMODEL: {
struct sound_trigger_model_event *soundmodelEvent =
(struct sound_trigger_model_event *)eventMemory->pointer();
- sp<ISoundTriggerClient> client;
{
AutoMutex lock(mLock);
sp<Model> model = getModel(soundmodelEvent->model);
@@ -786,29 +783,26 @@
ALOGW("%s model == 0", __func__);
return;
}
- client = model->mModuleClient->client();
- }
- if (client != 0) {
- client->onSoundModelEvent(eventMemory);
+ clients.add(model->mModuleClient);
}
} break;
case CallbackEvent::TYPE_SERVICE_STATE: {
- Vector< sp<ISoundTriggerClient> > clients;
{
AutoMutex lock(mLock);
for (size_t i = 0; i < mModuleClients.size(); i++) {
if (mModuleClients[i] != 0) {
- clients.add(mModuleClients[i]->client());
+ clients.add(mModuleClients[i]);
}
}
}
- for (size_t i = 0; i < clients.size(); i++) {
- clients[i]->onServiceStateChange(eventMemory);
- }
} break;
default:
LOG_ALWAYS_FATAL("onCallbackEvent unknown event type %d", event->mType);
}
+
+ for (size_t i = 0; i < clients.size(); i++) {
+ clients[i]->onCallbackEvent(event);
+ }
}
sp<SoundTriggerHwService::Model> SoundTriggerHwService::Module::getModel(
@@ -1070,19 +1064,26 @@
return;
}
- switch (event->mType) {
- case CallbackEvent::TYPE_SERVICE_STATE: {
- sp<ISoundTriggerClient> client;
- {
- AutoMutex lock(mLock);
- client = mClient;
- }
- if (client !=0 ) {
+ sp<ISoundTriggerClient> client;
+ {
+ AutoMutex lock(mLock);
+ client = mClient;
+ }
+
+ if (client != 0) {
+ switch (event->mType) {
+ case CallbackEvent::TYPE_RECOGNITION: {
+ client->onRecognitionEvent(eventMemory);
+ } break;
+ case CallbackEvent::TYPE_SOUNDMODEL: {
+ client->onSoundModelEvent(eventMemory);
+ } break;
+ case CallbackEvent::TYPE_SERVICE_STATE: {
client->onServiceStateChange(eventMemory);
+ } break;
+ default:
+ LOG_ALWAYS_FATAL("onCallbackEvent unknown event type %d", event->mType);
}
- } break;
- default:
- LOG_ALWAYS_FATAL("onCallbackEvent unknown event type %d", event->mType);
}
}