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

Commit 56ce2edb authored by Andy Hung's avatar Andy Hung
Browse files

AudioFlinger: defer restartIfDisabled() 

Defer restartIfDisabled() to the end of
the threadLoop worker thread, allowing execution
without any mutexes held. This prevents mutex
order issues.

To accomplish this, we add a DeferredExecutor object
to ThreadBase, which allows adding functors and dtors
to execute at the end of the worker loop,
where no mutexes are held.

Test: atest AudioPlaybackCaptureTest
Bug: 345400492
Bug: 345676143
Change-Id: I1d7f491fc1289882ce67cb7289b3bc1b58e81d23
parent 2718e019

services/audioflinger/IAfThread.h

+8 −1
Original line number Diff line number Diff line
@@ -19,8 +19,9 @@ 
#include <android/media/IAudioTrackCallback.h>

#include <android/media/IEffectClient.h>

#include <audiomanager/IAudioManager.h>

#include <audio_utils/mutex.h>

#include <audio_utils/DeferredExecutor.h>

#include <audio_utils/MelProcessor.h>

#include <audio_utils/mutex.h>

#include <binder/MemoryDealer.h>

#include <datapath/AudioStreamIn.h>

#include <datapath/AudioStreamOut.h>
@@ -394,6 +395,12 @@ public: 
    // avoid races.

    virtual void waitWhileThreadBusy_l(audio_utils::unique_lock& ul)

            REQUIRES(mutex()) = 0;



    // The ThreadloopExecutor is used to defer functors or dtors

    // to when the Threadloop does not hold any mutexes (at the end of the

    // processing period cycle).

    virtual audio_utils::DeferredExecutor& getThreadloopExecutor() = 0;



    // Dynamic cast to derived interface

    virtual sp<IAfDirectOutputThread> asIAfDirectOutputThread() { return nullptr; }

    virtual sp<IAfDuplicatingThread> asIAfDuplicatingThread() { return nullptr; }

services/audioflinger/PlaybackTracks.h

+2 −2
Original line number Diff line number Diff line
@@ -448,7 +448,7 @@ private: 
    void                queueBuffer(Buffer& inBuffer);

    void                clearBufferQueue();



    void                restartIfDisabled();

    void restartIfDisabled() override;



    // Maximum number of pending buffers allocated by OutputTrack::write()

    static const uint8_t kMaxOverFlowBuffers = 10;
@@ -512,7 +512,7 @@ public: 
    void releaseBuffer(Proxy::Buffer* buffer) final;



private:

            void restartIfDisabled();

    void restartIfDisabled() override;

};  // end of PatchTrack



} // namespace android

services/audioflinger/Threads.cpp

+10 −0
Original line number Diff line number Diff line
@@ -4582,7 +4582,11 @@ NO_THREAD_SAFETY_ANALYSIS // manual locking of AudioFlinger 


        // FIXME Note that the above .clear() is no longer necessary since effectChains

        // is now local to this block, but will keep it for now (at least until merge done).



        mThreadloopExecutor.process();

    }

    mThreadloopExecutor.process(); // process any remaining deferred actions.

    // deferred actions after this point are ignored.



    threadLoop_exit();
@@ -8732,11 +8736,14 @@ unlock: 
            mIoJitterMs.add(jitterMs);

            mProcessTimeMs.add(processMs);

        }

       mThreadloopExecutor.process();

        // update timing info.

        mLastIoBeginNs = lastIoBeginNs;

        mLastIoEndNs = lastIoEndNs;

        lastLoopCountRead = loopCount;

    }

    mThreadloopExecutor.process(); // process any remaining deferred actions.

    // deferred actions after this point are ignored.



    standbyIfNotAlreadyInStandby();
@@ -10522,7 +10529,10 @@ bool MmapThread::threadLoop() 
        unlockEffectChains(effectChains);

        // Effect chains will be actually deleted here if they were removed from

        // mEffectChains list during mixing or effects processing

        mThreadloopExecutor.process();

    }

    mThreadloopExecutor.process(); // process any remaining deferred actions.

    // deferred actions after this point are ignored.



    threadLoop_exit();

services/audioflinger/Threads.h

+12 −0
Original line number Diff line number Diff line
@@ -569,6 +569,10 @@ public: 
    void stopMelComputation_l() override

            REQUIRES(audio_utils::AudioFlinger_Mutex);



    audio_utils::DeferredExecutor& getThreadloopExecutor() override {

        return mThreadloopExecutor;

    }



protected:



                // entry describing an effect being suspended in mSuspendedSessions keyed vector
@@ -875,6 +879,14 @@ protected: 


                SimpleLog mLocalLog;  // locked internally



    // mThreadloopExecutor contains deferred functors and object (dtors) to

    // be executed at the end of the processing period, without any

    // mutexes held.

    //

    // mThreadloopExecutor is locked internally, so its methods are thread-safe

    // for access.

    audio_utils::DeferredExecutor mThreadloopExecutor;



    private:

    void dumpBase_l(int fd, const Vector<String16>& args) REQUIRES(mutex());

    void dumpEffectChains_l(int fd, const Vector<String16>& args) REQUIRES(mutex());

services/audioflinger/TrackBase.h

+3 −0
Original line number Diff line number Diff line
@@ -333,6 +333,9 @@ protected: 
                                    // true for Track, false for RecordTrack,

                                    // this could be a track type if needed later



    void deferRestartIfDisabled();

    virtual void restartIfDisabled() {}



    const wp<IAfThreadBase> mThread;

    const alloc_type     mAllocType;

    /*const*/ sp<Client> mClient;   // see explanation at ~TrackBase() why not const