PatchPanel: fix deadlock when creating a patch
In PatchPanel::createAudioPatch(), unlock AudioFlinger::mLock when calling
ThreadBase::sendCreateAudioPatchConfigEvent() to avoid deadlocks if the thread loop
needs to acquire AudioFlinger::mLock before processing the create patch request.
Also fix build with verbose log enabled in Threads.cpp.
Bug: 276386807
Test: repro setps in the bug
Change-Id: Iaa4a89588e75113316241373d3bdb6858c3893ba
diff --git a/services/audioflinger/AudioFlinger.h b/services/audioflinger/AudioFlinger.h
index 077fa26..323ce0e 100644
--- a/services/audioflinger/AudioFlinger.h
+++ b/services/audioflinger/AudioFlinger.h
@@ -425,6 +425,8 @@
bool btNrecIsOff() const { return mBtNrecIsOff.load(); }
+ void lock() ACQUIRE(mLock) { mLock.lock(); }
+ void unlock() RELEASE(mLock) { mLock.unlock(); }
private:
diff --git a/services/audioflinger/PatchPanel.cpp b/services/audioflinger/PatchPanel.cpp
index 3b428bb..d25d46f 100644
--- a/services/audioflinger/PatchPanel.cpp
+++ b/services/audioflinger/PatchPanel.cpp
@@ -135,6 +135,10 @@
status_t AudioFlinger::PatchPanel::createAudioPatch(const struct audio_patch *patch,
audio_patch_handle_t *handle,
bool endpointPatch)
+ //unlocks AudioFlinger::mLock when calling ThreadBase::sendCreateAudioPatchConfigEvent
+ //to avoid deadlocks if the thread loop needs to acquire AudioFlinger::mLock
+ //before processing the create patch request.
+ NO_THREAD_SAFETY_ANALYSIS
{
if (handle == NULL || patch == NULL) {
return BAD_VALUE;
@@ -245,7 +249,6 @@
status = INVALID_OPERATION;
goto exit;
}
-
sp<ThreadBase> thread =
mAudioFlinger.checkPlaybackThread_l(patch->sources[1].ext.mix.handle);
if (thread == 0) {
@@ -356,11 +359,12 @@
goto exit;
}
}
+ mAudioFlinger.unlock();
status = thread->sendCreateAudioPatchConfigEvent(patch, &halHandle);
+ mAudioFlinger.lock();
if (status == NO_ERROR) {
newPatch.setThread(thread);
}
-
// remove stale audio patch with same input as sink if any
for (auto& iter : mPatches) {
if (iter.second.mAudioPatch.sinks[0].ext.mix.handle == thread->id()) {
@@ -422,7 +426,9 @@
mAudioFlinger.updateOutDevicesForRecordThreads_l(devices);
}
+ mAudioFlinger.unlock();
status = thread->sendCreateAudioPatchConfigEvent(patch, &halHandle);
+ mAudioFlinger.lock();
if (status == NO_ERROR) {
newPatch.setThread(thread);
}
diff --git a/services/audioflinger/Threads.cpp b/services/audioflinger/Threads.cpp
index 76c9ad8..de0abf0 100644
--- a/services/audioflinger/Threads.cpp
+++ b/services/audioflinger/Threads.cpp
@@ -4976,7 +4976,7 @@
const NBAIO_Format offersFast[1] = {format};
size_t numCounterOffersFast = 0;
#if !LOG_NDEBUG
- ssize_t index =
+ index =
#else
(void)
#endif
@@ -7874,15 +7874,15 @@
Pipe *pipe = new Pipe(pipeFramesP2, format, pipeBuffer);
const NBAIO_Format offersFast[1] = {format};
size_t numCounterOffersFast = 0;
- [[maybe_unused]] ssize_t index = pipe->negotiate(offersFast, std::size(offersFast),
+ [[maybe_unused]] ssize_t index2 = pipe->negotiate(offersFast, std::size(offersFast),
nullptr /* counterOffers */, numCounterOffersFast);
- ALOG_ASSERT(index == 0);
+ ALOG_ASSERT(index2 == 0);
mPipeSink = pipe;
PipeReader *pipeReader = new PipeReader(*pipe);
numCounterOffersFast = 0;
- index = pipeReader->negotiate(offersFast, std::size(offersFast),
+ index2 = pipeReader->negotiate(offersFast, std::size(offersFast),
nullptr /* counterOffers */, numCounterOffersFast);
- ALOG_ASSERT(index == 0);
+ ALOG_ASSERT(index2 == 0);
mPipeSource = pipeReader;
mPipeFramesP2 = pipeFramesP2;
mPipeMemory = pipeMemory;
@@ -9297,7 +9297,7 @@
if (stepCount == 0) {
return;
}
- ALOG_ASSERT(stepCount <= mRsmpInUnrel);
+ ALOG_ASSERT(stepCount <= (int32_t)mRsmpInUnrel);
mRsmpInUnrel -= stepCount;
mRsmpInFront = audio_utils::safe_add_overflow(mRsmpInFront, stepCount);
buffer->raw = NULL;