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

Commit 1ea6d233 authored by Glenn Kasten's avatar Glenn Kasten
Browse files

Use atomic ops for thread suspend count 

There was a theoretical but unlikely race if two binder threads
executed suspend() or restore() concurrently.  Also added comments.

Change-Id: I0908acc810b83bdd66455b27ca3429de1662a2cd
parent 68337edf

services/audioflinger/AudioFlinger.cpp

+3 −3
Original line number Diff line number Diff line
@@ -2513,7 +2513,7 @@ bool AudioFlinger::PlaybackThread::threadLoop() 


            // put audio hardware into standby after short delay

            if (CC_UNLIKELY((!mActiveTracks.size() && systemTime() > standbyTime) ||

                        mSuspended > 0)) {

                        isSuspended())) {

                if (!mStandby) {



                    threadLoop_standby();
@@ -2567,7 +2567,7 @@ bool AudioFlinger::PlaybackThread::threadLoop() 
            threadLoop_sleepTime();

        }



        if (mSuspended > 0) {

        if (isSuspended()) {

            sleepTime = suspendSleepTimeUs();

        }
@@ -2752,7 +2752,7 @@ void AudioFlinger::MixerThread::threadLoop_standby() 
// shared by MIXER and DIRECT, overridden by DUPLICATING

void AudioFlinger::PlaybackThread::threadLoop_standby()

{

    ALOGV("Audio hardware entering standby, mixer %p, suspend count %u", this, mSuspended);

    ALOGV("Audio hardware entering standby, mixer %p, suspend count %d", this, mSuspended);

    mOutput->stream->common.standby(&mOutput->stream->common);

}

services/audioflinger/AudioFlinger.h

+21 −4
Original line number Diff line number Diff line
@@ -1023,9 +1023,19 @@ public: 
                    AudioStreamOut* clearOutput();

                    virtual audio_stream_t* stream() const;



                    void        suspend() { mSuspended++; }

                    void        restore() { if (mSuspended > 0) mSuspended--; }

                    bool        isSuspended() const { return (mSuspended > 0); }

                    // a very large number of suspend() will eventually wraparound, but unlikely

                    void        suspend() { (void) android_atomic_inc(&mSuspended); }

                    void        restore()

                                    {

                                        // if restore() is done without suspend(), get back into

                                        // range so that the next suspend() will operate correctly

                                        if (android_atomic_dec(&mSuspended) <= 0) {

                                            android_atomic_release_store(0, &mSuspended);

                                        }

                                    }

                    bool        isSuspended() const

                                    { return android_atomic_acquire_load(&mSuspended) > 0; }



        virtual     String8     getParameters(const String8& keys);

        virtual     void        audioConfigChanged_l(int event, int param = 0);

                    status_t    getRenderPosition(uint32_t *halFrames, uint32_t *dspFrames);
@@ -1050,7 +1060,14 @@ public: 


    protected:

        int16_t*                        mMixBuffer;

        uint32_t                        mSuspended;     // suspend count, > 0 means suspended



        // suspend count, > 0 means suspended.  While suspended, the thread continues to pull from

        // tracks and mix, but doesn't write to HAL.  A2DP and SCO HAL implementations can't handle

        // concurrent use of both of them, so Audio Policy Service suspends one of the threads to

        // workaround that restriction.

        // 'volatile' means accessed via atomic operations and no lock.

        volatile int32_t                mSuspended;



        int                             mBytesWritten;

    private:

        // mMasterMute is in both PlaybackThread and in AudioFlinger.  When a