Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit d585959c authored by Vlad Popa's avatar Vlad Popa
Browse files

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
Change-Id: I16555608176f1b1dfa180e82346319b55c19de01
parent 10e1b68d

services/audioflinger/PlaybackTracks.h

+1 −1
Original line number Diff line number Diff line
@@ -58,7 +58,7 @@ private: 


    sp<PlayAudioOpCallback> mOpCallback;

    // called by PlayAudioOpCallback when OP_PLAY_AUDIO is updated in AppOp callback

    void checkPlayAudioForUsage();

    void checkPlayAudioForUsage(bool doBroadcast);



    wp<IAfThreadBase> mThread;

    std::atomic_bool mHasOpPlayAudio;

services/audioflinger/Tracks.cpp

+12 −8
Original line number Diff line number Diff line
@@ -611,7 +611,9 @@ OpPlayAudioMonitor::~OpPlayAudioMonitor() 


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,
@@ -626,7 +628,7 @@ bool OpPlayAudioMonitor::hasOpPlayAudio() const { 
// 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(
@@ -636,6 +638,7 @@ void OpPlayAudioMonitor::checkPlayAudioForUsage() 
    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 " : "");

        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.
@@ -644,6 +647,7 @@ void OpPlayAudioMonitor::checkPlayAudioForUsage() 
            }

        }

    }

}



OpPlayAudioMonitor::PlayAudioOpCallback::PlayAudioOpCallback(

        const wp<OpPlayAudioMonitor>& monitor) : mMonitor(monitor)
@@ -658,7 +662,7 @@ void OpPlayAudioMonitor::PlayAudioOpCallback::opChanged(int32_t op, 
    }

    sp<OpPlayAudioMonitor> monitor = mMonitor.promote();

    if (monitor != NULL) {

        monitor->checkPlayAudioForUsage();

        monitor->checkPlayAudioForUsage(/*doBroadcast=*/true);

    }

}