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

2 files changed, 24 insertions, 7 deletions

diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp
index fc4969e..e2b29f7 100644
--- a/services/audioflinger/AudioFlinger.cpp
+++ b/services/audioflinger/AudioFlinger.cpp
@@ -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);
 }
 
diff --git a/services/audioflinger/AudioFlinger.h b/services/audioflinger/AudioFlinger.h
index 648a8d2..a6fd0a0 100644
--- a/services/audioflinger/AudioFlinger.h
+++ b/services/audioflinger/AudioFlinger.h
@@ -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