Soundtrigger service: fix cross deadlock with audio policy service
Do not hold Module mutex when calling into audio policy manager to
avoid cross deadlock with audio poicy service mutex: Audio policy manager
can call into sound trigger service with its mutex held in methods like
stopInput().
Regression introduced by fix for b/64340921 commit f759b8c4
Bug: 64340921
Bug: 67310830
Test: repro steps in b/67310830
Merged-In: Ie50b2e7c55fe9828a3fd8de6b31eb4a492791583
Change-Id: Ie50b2e7c55fe9828a3fd8de6b31eb4a492791583
diff --git a/services/audiopolicy/common/managerdefinitions/src/SoundTriggerSession.cpp b/services/audiopolicy/common/managerdefinitions/src/SoundTriggerSession.cpp
index 8ca3ae0..0383b0e 100644
--- a/services/audiopolicy/common/managerdefinitions/src/SoundTriggerSession.cpp
+++ b/services/audiopolicy/common/managerdefinitions/src/SoundTriggerSession.cpp
@@ -34,7 +34,7 @@
{
ssize_t index = indexOfKey(session);
if (index < 0) {
- ALOGW("acquireSoundTriggerSession() session %d not registered", session);
+ ALOGW("releaseSession() session %d not registered", session);
return BAD_VALUE;
}
diff --git a/services/soundtrigger/SoundTriggerHwService.cpp b/services/soundtrigger/SoundTriggerHwService.cpp
index 5b8d990..19dd41e 100644
--- a/services/soundtrigger/SoundTriggerHwService.cpp
+++ b/services/soundtrigger/SoundTriggerHwService.cpp
@@ -528,37 +528,45 @@
void SoundTriggerHwService::Module::detach(const sp<ModuleClient>& moduleClient)
{
ALOGV("Module::detach()");
- AutoMutex lock(mLock);
- ssize_t index = -1;
+ Vector<audio_session_t> releasedSessions;
- for (size_t i = 0; i < mModuleClients.size(); i++) {
- if (mModuleClients[i] == moduleClient) {
- index = i;
- break;
- }
- }
- if (index == -1) {
- return;
- }
+ {
+ AutoMutex lock(mLock);
+ ssize_t index = -1;
- ALOGV("remove client %p", moduleClient.get());
- mModuleClients.removeAt(index);
-
- // Iterate in reverse order as models are removed from list inside the loop.
- for (size_t i = mModels.size(); i > 0; i--) {
- sp<Model> model = mModels.valueAt(i - 1);
- if (moduleClient == model->mModuleClient) {
- mModels.removeItemsAt(i - 1);
- ALOGV("detach() unloading model %d", model->mHandle);
- if (mHalInterface != 0) {
- if (model->mState == Model::STATE_ACTIVE) {
- mHalInterface->stopRecognition(model->mHandle);
- }
- mHalInterface->unloadSoundModel(model->mHandle);
+ for (size_t i = 0; i < mModuleClients.size(); i++) {
+ if (mModuleClients[i] == moduleClient) {
+ index = i;
+ break;
}
- AudioSystem::releaseSoundTriggerSession(model->mCaptureSession);
- mHalInterface->unloadSoundModel(model->mHandle);
}
+ if (index == -1) {
+ return;
+ }
+
+ ALOGV("remove client %p", moduleClient.get());
+ mModuleClients.removeAt(index);
+
+ // Iterate in reverse order as models are removed from list inside the loop.
+ for (size_t i = mModels.size(); i > 0; i--) {
+ sp<Model> model = mModels.valueAt(i - 1);
+ if (moduleClient == model->mModuleClient) {
+ mModels.removeItemsAt(i - 1);
+ ALOGV("detach() unloading model %d", model->mHandle);
+ if (mHalInterface != 0) {
+ if (model->mState == Model::STATE_ACTIVE) {
+ mHalInterface->stopRecognition(model->mHandle);
+ }
+ mHalInterface->unloadSoundModel(model->mHandle);
+ }
+ releasedSessions.add(model->mCaptureSession);
+ }
+ }
+ }
+
+ for (size_t i = 0; i < releasedSessions.size(); i++) {
+ // do not call AudioSystem methods with mLock held
+ AudioSystem::releaseSoundTriggerSession(releasedSessions[i]);
}
}
@@ -593,61 +601,71 @@
return BAD_VALUE;
}
- AutoMutex lock(mLock);
-
- if (mModels.size() >= mDescriptor.properties.max_sound_models) {
- ALOGW("loadSoundModel(): Not loading, max number of models (%d) would be exceeded",
- mDescriptor.properties.max_sound_models);
- return INVALID_OPERATION;
- }
-
- status_t status = mHalInterface->loadSoundModel(sound_model,
- SoundTriggerHwService::soundModelCallback,
- this, handle);
-
- if (status != NO_ERROR) {
- return status;
- }
audio_session_t session;
audio_io_handle_t ioHandle;
audio_devices_t device;
-
- status = AudioSystem::acquireSoundTriggerSession(&session, &ioHandle, &device);
+ // do not call AudioSystem methods with mLock held
+ status_t status = AudioSystem::acquireSoundTriggerSession(&session, &ioHandle, &device);
if (status != NO_ERROR) {
return status;
}
- sp<Model> model = new Model(*handle, session, ioHandle, device, sound_model->type,
- moduleClient);
- mModels.replaceValueFor(*handle, model);
+ {
+ AutoMutex lock(mLock);
+ if (mModels.size() >= mDescriptor.properties.max_sound_models) {
+ ALOGW("loadSoundModel(): Not loading, max number of models (%d) would be exceeded",
+ mDescriptor.properties.max_sound_models);
+ status = INVALID_OPERATION;
+ goto exit;
+ }
+
+ status = mHalInterface->loadSoundModel(sound_model,
+ SoundTriggerHwService::soundModelCallback,
+ this, handle);
+ if (status != NO_ERROR) {
+ goto exit;
+ }
+
+ sp<Model> model = new Model(*handle, session, ioHandle, device, sound_model->type,
+ moduleClient);
+ mModels.replaceValueFor(*handle, model);
+ }
+exit:
+ if (status != NO_ERROR) {
+ // do not call AudioSystem methods with mLock held
+ AudioSystem::releaseSoundTriggerSession(session);
+ }
return status;
}
status_t SoundTriggerHwService::Module::unloadSoundModel(sound_model_handle_t handle)
{
ALOGV("unloadSoundModel() model handle %d", handle);
- AutoMutex lock(mLock);
- return unloadSoundModel_l(handle);
-}
+ status_t status;
+ audio_session_t session;
-status_t SoundTriggerHwService::Module::unloadSoundModel_l(sound_model_handle_t handle)
-{
- if (mHalInterface == 0) {
- return NO_INIT;
+ {
+ AutoMutex lock(mLock);
+ if (mHalInterface == 0) {
+ return NO_INIT;
+ }
+ ssize_t index = mModels.indexOfKey(handle);
+ if (index < 0) {
+ return BAD_VALUE;
+ }
+ sp<Model> model = mModels.valueAt(index);
+ mModels.removeItem(handle);
+ if (model->mState == Model::STATE_ACTIVE) {
+ mHalInterface->stopRecognition(model->mHandle);
+ model->mState = Model::STATE_IDLE;
+ }
+ status = mHalInterface->unloadSoundModel(handle);
+ session = model->mCaptureSession;
}
- ssize_t index = mModels.indexOfKey(handle);
- if (index < 0) {
- return BAD_VALUE;
- }
- sp<Model> model = mModels.valueAt(index);
- mModels.removeItem(handle);
- if (model->mState == Model::STATE_ACTIVE) {
- mHalInterface->stopRecognition(model->mHandle);
- model->mState = Model::STATE_IDLE;
- }
- AudioSystem::releaseSoundTriggerSession(model->mCaptureSession);
- return mHalInterface->unloadSoundModel(handle);
+ // do not call AudioSystem methods with mLock held
+ AudioSystem::releaseSoundTriggerSession(session);
+ return status;
}
status_t SoundTriggerHwService::Module::startRecognition(sound_model_handle_t handle,
diff --git a/services/soundtrigger/SoundTriggerHwService.h b/services/soundtrigger/SoundTriggerHwService.h
index 60ebb35..95efc4b 100644
--- a/services/soundtrigger/SoundTriggerHwService.h
+++ b/services/soundtrigger/SoundTriggerHwService.h
@@ -140,8 +140,6 @@
private:
- status_t unloadSoundModel_l(sound_model_handle_t handle);
-
Mutex mLock;
wp<SoundTriggerHwService> mService;
sp<SoundTriggerHalInterface> mHalInterface;
