From e00e82e19e828c3a23fa8e0f888d433d53bc7357 Mon Sep 17 00:00:00 2001 From: Eric Laurent Date: Thu, 5 Oct 2017 10:58:38 -0700 Subject: 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 --- services/soundtrigger/SoundTriggerHwService.cpp | 87 +++++++++++++++++-------- services/soundtrigger/SoundTriggerHwService.h | 2 +- 2 files changed, 61 insertions(+), 28 deletions(-) (limited to 'services') diff --git a/services/soundtrigger/SoundTriggerHwService.cpp b/services/soundtrigger/SoundTriggerHwService.cpp index 665672f184..5ae38625b5 100644 --- a/services/soundtrigger/SoundTriggerHwService.cpp +++ b/services/soundtrigger/SoundTriggerHwService.cpp @@ -497,6 +497,8 @@ void SoundTriggerHwService::Module::detach() { if (!captureHotwordAllowed()) { return; } + Vector releasedSessions; + { AutoMutex lock(mLock); for (size_t i = 0; i < mModels.size(); i++) { @@ -506,9 +508,16 @@ void SoundTriggerHwService::Module::detach() { mHwDevice->stop_recognition(mHwDevice, model->mHandle); } mHwDevice->unload_sound_model(mHwDevice, model->mHandle); + releasedSessions.add(model->mCaptureSession); } mModels.clear(); } + + for (size_t i = 0; i < releasedSessions.size(); i++) { + // do not call AudioSystem methods with mLock held + AudioSystem::releaseSoundTriggerSession(releasedSessions[i]); + } + if (mClient != 0) { mClient->asBinder()->unlinkToDeath(this); } @@ -550,37 +559,52 @@ status_t SoundTriggerHwService::Module::loadSoundModel(const sp& modelM return BAD_VALUE; } - AutoMutex lock(mLock); - - if (mModels.size() >= mDescriptor.properties.max_sound_models) { - if (mModels.size() == 0) { - return INVALID_OPERATION; - } - ALOGW("loadSoundModel() max number of models exceeded %d making room for a new one", - mDescriptor.properties.max_sound_models); - unloadSoundModel_l(mModels.valueAt(0)->mHandle); - } - - status_t status = mHwDevice->load_sound_model(mHwDevice, - sound_model, - SoundTriggerHwService::soundModelCallback, - this, - handle); - if (status != NO_ERROR) { - return status; - } audio_session_t session; + audio_session_t freedSession = AUDIO_SESSION_ALLOCATE; 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 = new Model(*handle, session, ioHandle, device, sound_model->type); - mModels.replaceValueFor(*handle, model); + { + AutoMutex lock(mLock); + + + if (mModels.size() >= mDescriptor.properties.max_sound_models) { + if (mModels.size() == 0) { + status = INVALID_OPERATION; + goto exit; + } + ALOGW("loadSoundModel() max number of models exceeded %d making room for a new one", + mDescriptor.properties.max_sound_models); + unloadSoundModel_l(mModels.valueAt(0)->mHandle, &freedSession); + } + + status_t status = mHwDevice->load_sound_model(mHwDevice, + sound_model, + SoundTriggerHwService::soundModelCallback, + this, + handle); + if (status != NO_ERROR) { + goto exit; + } + sp model = new Model(*handle, session, ioHandle, device, sound_model->type); + mModels.replaceValueFor(*handle, model); + } + +exit: + if (freedSession != AUDIO_SESSION_ALLOCATE) { + // do not call AudioSystem methods with mLock held + AudioSystem::releaseSoundTriggerSession(freedSession); + } + if (status != NO_ERROR) { + // do not call AudioSystem methods with mLock held + AudioSystem::releaseSoundTriggerSession(session); + } return status; } @@ -591,12 +615,21 @@ status_t SoundTriggerHwService::Module::unloadSoundModel(sound_model_handle_t ha return PERMISSION_DENIED; } - AutoMutex lock(mLock); - return unloadSoundModel_l(handle); + status_t status; + audio_session_t freedSession = AUDIO_SESSION_ALLOCATE; + { + AutoMutex lock(mLock); + status = unloadSoundModel_l(handle, &freedSession); + } + if (freedSession != AUDIO_SESSION_ALLOCATE) { + AudioSystem::releaseSoundTriggerSession(freedSession); + } + return status; } -status_t SoundTriggerHwService::Module::unloadSoundModel_l(sound_model_handle_t handle) +status_t SoundTriggerHwService::Module::unloadSoundModel_l(sound_model_handle_t handle, audio_session_t *session) { + *session = AUDIO_SESSION_ALLOCATE; ssize_t index = mModels.indexOfKey(handle); if (index < 0) { return BAD_VALUE; @@ -607,7 +640,7 @@ status_t SoundTriggerHwService::Module::unloadSoundModel_l(sound_model_handle_t mHwDevice->stop_recognition(mHwDevice, model->mHandle); model->mState = Model::STATE_IDLE; } - AudioSystem::releaseSoundTriggerSession(model->mCaptureSession); + *session = model->mCaptureSession; return mHwDevice->unload_sound_model(mHwDevice, handle); } diff --git a/services/soundtrigger/SoundTriggerHwService.h b/services/soundtrigger/SoundTriggerHwService.h index 2619a5fb49..f0e7a5fc5f 100644 --- a/services/soundtrigger/SoundTriggerHwService.h +++ b/services/soundtrigger/SoundTriggerHwService.h @@ -141,7 +141,7 @@ public: private: - status_t unloadSoundModel_l(sound_model_handle_t handle); + status_t unloadSoundModel_l(sound_model_handle_t handle, audio_session_t *session); Mutex mLock; -- cgit v1.2.3