AudioFlinger: Update Track / Thread mutex handling
Prevent deadlock when removing track from active tracks.
Test: atest CtsMediaAudioTestCases (including AudioPlaybackCaptureTest)
Test: Spatializer playback.
Test: Duplicating playback BT/Speaker.
Bug: 314229445
Merged-In: I802759430cec782cdefba89ca2bbafb9af67588c
Change-Id: I802759430cec782cdefba89ca2bbafb9af67588c
diff --git a/services/audioflinger/Threads.cpp b/services/audioflinger/Threads.cpp
index d61621a..ddef7f3 100644
--- a/services/audioflinger/Threads.cpp
+++ b/services/audioflinger/Threads.cpp
@@ -2849,6 +2849,8 @@
// effectively get the latency it requested.
if (track->isExternalTrack()) {
IAfTrackBase::track_state state = track->state();
+ // Because the track is not on the ActiveTracks,
+ // at this point, only the TrackHandle will be adding the track.
mutex().unlock();
status = AudioSystem::startOutput(track->portId());
mutex().lock();
@@ -2929,7 +2931,12 @@
track->setResetDone(false);
track->resetPresentationComplete();
+
+ // Do not release the ThreadBase mutex after the track is added to mActiveTracks unless
+ // all key changes are complete. It is possible that the threadLoop will begin
+ // processing the added track immediately after the ThreadBase mutex is released.
mActiveTracks.add(track);
+
if (chain != 0) {
ALOGV("addTrack_l() starting track on chain %p for session %d", chain.get(),
track->sessionId());
@@ -4704,8 +4711,12 @@
void PlaybackThread::removeTracks_l(const Vector<sp<IAfTrack>>& tracksToRemove)
NO_THREAD_SAFETY_ANALYSIS // release and re-acquire mutex()
{
+ if (tracksToRemove.empty()) return;
+
+ // Block all incoming TrackHandle requests until we are finished with the release.
+ setThreadBusy_l(true);
+
for (const auto& track : tracksToRemove) {
- mActiveTracks.remove(track);
ALOGV("%s(%d): removing track on session %d", __func__, track->id(), track->sessionId());
sp<IAfEffectChain> chain = getEffectChain_l(track->sessionId());
if (chain != 0) {
@@ -4713,17 +4724,16 @@
__func__, track->id(), chain.get(), track->sessionId());
chain->decActiveTrackCnt();
}
+
// If an external client track, inform APM we're no longer active, and remove if needed.
- // We do this under lock so that the state is consistent if the Track is destroyed.
+ // Since the track is active, we do it here instead of TrackBase::destroy().
if (track->isExternalTrack()) {
+ mutex().unlock();
AudioSystem::stopOutput(track->portId());
if (track->isTerminated()) {
AudioSystem::releaseOutput(track->portId());
}
- }
- if (track->isTerminated()) {
- // remove from our tracks vector
- removeTrack_l(track);
+ mutex().lock();
}
if (mHapticChannelCount > 0 &&
((track->channelMask() & AUDIO_CHANNEL_HAPTIC_ALL) != AUDIO_CHANNEL_NONE
@@ -4740,7 +4750,24 @@
chain->setHapticIntensity_l(track->id(), os::HapticScale::MUTE);
}
}
+
+ // Under lock, the track is removed from the active tracks list.
+ //
+ // Once the track is no longer active, the TrackHandle may directly
+ // modify it as the threadLoop() is no longer responsible for its maintenance.
+ // Do not modify the track from threadLoop after the mutex is unlocked
+ // if it is not active.
+ mActiveTracks.remove(track);
+
+ if (track->isTerminated()) {
+ // remove from our tracks vector
+ removeTrack_l(track);
+ }
}
+
+ // Allow incoming TrackHandle requests. We still hold the mutex,
+ // so pending TrackHandle requests will occur after we unlock it.
+ setThreadBusy_l(false);
}
status_t PlaybackThread::getTimestamp_l(AudioTimestamp& timestamp)