From 1fb6c97d6cf1ae3264605d912b4e034809e2103a Mon Sep 17 00:00:00 2001 From: Haynes Mathew George Date: Mon, 26 Oct 2015 18:22:13 -0700 Subject: AudioPolicyService: Synchronize access to AudioPolicyManager Synchronize access to APM when getDevicesForStream is called on APM. CRs-Fixed: 913227 Change-Id: I2ba6922341f035375270b02000ef5a7e078f6b5a --- services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp | 1 + 1 file changed, 1 insertion(+) (limited to 'services/audiopolicy') diff --git a/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp b/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp index 793c26a..58ecb11 100644 --- a/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp +++ b/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp @@ -459,6 +459,7 @@ audio_devices_t AudioPolicyService::getDevicesForStream(audio_stream_type_t stre if (mAudioPolicyManager == NULL) { return AUDIO_DEVICE_NONE; } + Mutex::Autolock _l(mLock); return mAudioPolicyManager->getDevicesForStream(stream); } -- cgit v1.1 From f17204ea09e5629f3f9d9eba2e66d9fb6ce72357 Mon Sep 17 00:00:00 2001 From: Eric Laurent Date: Wed, 23 Sep 2015 10:25:31 -0700 Subject: AudioPolicyService: fix race in AudioCommandThread Fixe race condition in AudioCommandThread::threadLoop() where a command can be inserted in first position in the queue after the sleep time has been calculated causing a longer delay than expected. Also fix a failure to hold a wake lock while commands are still in the queue. Bug: 22707905. Change-Id: I813626986677bf00106acb37ee20d3dd75d5cf33 --- .../audiopolicy/service/AudioPolicyService.cpp | 24 ++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) (limited to 'services/audiopolicy') diff --git a/services/audiopolicy/service/AudioPolicyService.cpp b/services/audiopolicy/service/AudioPolicyService.cpp index d689065..41dd40c 100644 --- a/services/audiopolicy/service/AudioPolicyService.cpp +++ b/services/audiopolicy/service/AudioPolicyService.cpp @@ -577,22 +577,28 @@ bool AudioPolicyService::AudioCommandThread::threadLoop() } } waitTime = INT64_MAX; + // release mLock before releasing strong reference on the service as + // AudioPolicyService destructor calls AudioCommandThread::exit() which + // acquires mLock. + mLock.unlock(); + svc.clear(); + mLock.lock(); } else { waitTime = mAudioCommands[0]->mTime - curTime; break; } } - // release mLock before releasing strong reference on the service as - // AudioPolicyService destructor calls AudioCommandThread::exit() which acquires mLock. - mLock.unlock(); - svc.clear(); - mLock.lock(); - if (!exitPending() && (mAudioCommands.isEmpty() || waitTime != INT64_MAX)) { - // release delayed commands wake lock + + // release delayed commands wake lock if the queue is empty + if (mAudioCommands.isEmpty()) { release_wake_lock(mName.string()); + } + + // At this stage we have either an empty command queue or the first command in the queue + // has a finite delay. So unless we are exiting it is safe to wait. + if (!exitPending()) { ALOGV("AudioCommandThread() going to sleep"); mWaitWorkCV.waitRelative(mLock, waitTime); - ALOGV("AudioCommandThread() waking up"); } } // release delayed commands wake lock before quitting @@ -1005,6 +1011,8 @@ void AudioPolicyService::AudioCommandThread::exit() requestExit(); mWaitWorkCV.signal(); } + // Note that we can call it from the thread loop if all other references have been released + // but it will safely return WOULD_BLOCK in this case requestExitAndWait(); } -- cgit v1.1 From 614ee35203329c5a617d0c5cfca9cd08446c3af5 Mon Sep 17 00:00:00 2001 From: Haynes Mathew George Date: Fri, 30 Oct 2015 15:29:47 -0700 Subject: audiopolicy: Add synchronization to EffectDescriptorCollection Synchronize public APIs of EffectDescriptorCollection CRs-Fixed: 920103 Change-Id: I04ccac526c6f99e61e43288776653d6b7ff325c4 --- .../audiopolicy/common/managerdefinitions/include/EffectDescriptor.h | 3 +++ .../audiopolicy/common/managerdefinitions/src/EffectDescriptor.cpp | 5 +++++ 2 files changed, 8 insertions(+) (limited to 'services/audiopolicy') diff --git a/services/audiopolicy/common/managerdefinitions/include/EffectDescriptor.h b/services/audiopolicy/common/managerdefinitions/include/EffectDescriptor.h index c9783a1..396541b 100644 --- a/services/audiopolicy/common/managerdefinitions/include/EffectDescriptor.h +++ b/services/audiopolicy/common/managerdefinitions/include/EffectDescriptor.h @@ -21,6 +21,7 @@ #include #include #include +#include namespace android { @@ -66,6 +67,8 @@ private: * Maximum memory allocated to audio effects in KB */ static const uint32_t MAX_EFFECTS_MEMORY = 512; + + Mutex mLock; }; }; // namespace android diff --git a/services/audiopolicy/common/managerdefinitions/src/EffectDescriptor.cpp b/services/audiopolicy/common/managerdefinitions/src/EffectDescriptor.cpp index 33d838d..6a0d079 100644 --- a/services/audiopolicy/common/managerdefinitions/src/EffectDescriptor.cpp +++ b/services/audiopolicy/common/managerdefinitions/src/EffectDescriptor.cpp @@ -56,6 +56,7 @@ status_t EffectDescriptorCollection::registerEffect(const effect_descriptor_t *d int session, int id) { + Mutex::Autolock _l(mLock); if (mTotalEffectsMemory + desc->memoryUsage > getMaxEffectsMemory()) { ALOGW("registerEffect() memory limit exceeded for Fx %s, Memory %d KB", desc->name, desc->memoryUsage); @@ -80,6 +81,7 @@ status_t EffectDescriptorCollection::registerEffect(const effect_descriptor_t *d status_t EffectDescriptorCollection::unregisterEffect(int id) { + Mutex::Autolock _l(mLock); ssize_t index = indexOfKey(id); if (index < 0) { ALOGW("unregisterEffect() unknown effect ID %d", id); @@ -106,6 +108,7 @@ status_t EffectDescriptorCollection::unregisterEffect(int id) status_t EffectDescriptorCollection::setEffectEnabled(int id, bool enabled) { + Mutex::Autolock _l(mLock); ssize_t index = indexOfKey(id); if (index < 0) { ALOGW("unregisterEffect() unknown effect ID %d", id); @@ -148,6 +151,7 @@ status_t EffectDescriptorCollection::setEffectEnabled(const sp bool EffectDescriptorCollection::isNonOffloadableEffectEnabled() { + Mutex::Autolock _l(mLock); for (size_t i = 0; i < size(); i++) { sp effectDesc = valueAt(i); if (effectDesc->mEnabled && (effectDesc->mStrategy == STRATEGY_MEDIA) && @@ -172,6 +176,7 @@ uint32_t EffectDescriptorCollection::getMaxEffectsMemory() const status_t EffectDescriptorCollection::dump(int fd) { + Mutex::Autolock _l(mLock); const size_t SIZE = 256; char buffer[SIZE]; -- cgit v1.1