APC: Add possible deadlock fix when creating track
When creating a new track the thread lock is being held. This can have
the effect of causing a deadlock when the OpPlayAudioMonitor is being
created since the onFirstRef method could also try to access the same
thread lock. Removing the broadcast when calling onFirstRef which is not
needed and would also avoid the deadlock
Test: atest AudioPlaybackConfigurationTest
Bug: 289885996
Merged-In: I16555608176f1b1dfa180e82346319b55c19de01
Change-Id: I16555608176f1b1dfa180e82346319b55c19de01
(cherry picked from commit d585959ccce748ba024c63d09b08d737eabfc70a)
diff --git a/services/audioflinger/Tracks.cpp b/services/audioflinger/Tracks.cpp
index 9169783..775aea6 100644
--- a/services/audioflinger/Tracks.cpp
+++ b/services/audioflinger/Tracks.cpp
@@ -610,7 +610,9 @@
void OpPlayAudioMonitor::onFirstRef()
{
- checkPlayAudioForUsage();
+ // make sure not to broadcast the initial state since it is not needed and could
+ // cause a deadlock since this method can be called with the mThread->mLock held
+ checkPlayAudioForUsage(/*doBroadcast=*/false);
if (mAttributionSource.packageName.has_value()) {
mOpCallback = new PlayAudioOpCallback(this);
mAppOpsManager.startWatchingMode(AppOpsManager::OP_PLAY_AUDIO,
@@ -625,7 +627,7 @@
// Note this method is never called (and never to be) for audio server / patch record track
// - not called from constructor due to check on UID,
// - not called from PlayAudioOpCallback because the callback is not installed in this case
-void OpPlayAudioMonitor::checkPlayAudioForUsage()
+void OpPlayAudioMonitor::checkPlayAudioForUsage(bool doBroadcast)
{
const bool hasAppOps = mAttributionSource.packageName.has_value()
&& mAppOpsManager.checkAudioOpNoThrow(
@@ -635,11 +637,13 @@
bool shouldChange = !hasAppOps; // check if we need to update.
if (mHasOpPlayAudio.compare_exchange_strong(shouldChange, hasAppOps)) {
ALOGD("OpPlayAudio: track:%d usage:%d %smuted", mId, mUsage, hasAppOps ? "not " : "");
- auto thread = mThread.promote();
- if (thread != nullptr && thread->type() == IAfThreadBase::OFFLOAD) {
- // Wake up Thread if offloaded, otherwise it may be several seconds for update.
- Mutex::Autolock _l(thread->mutex());
- thread->broadcast_l();
+ if (doBroadcast) {
+ auto thread = mThread.promote();
+ if (thread != nullptr && thread->type() == IAfThreadBase::OFFLOAD) {
+ // Wake up Thread if offloaded, otherwise it may be several seconds for update.
+ Mutex::Autolock _l(thread->mutex());
+ thread->broadcast_l();
+ }
}
}
}
@@ -657,7 +661,7 @@
}
sp<OpPlayAudioMonitor> monitor = mMonitor.promote();
if (monitor != NULL) {
- monitor->checkPlayAudioForUsage();
+ monitor->checkPlayAudioForUsage(/*doBroadcast=*/true);
}
}