commit b86432b1d62e7c9034223f0120b9bceb44215c41
author Glenn Kasten <gkasten@google.com> Wed Aug 14 15:08:12 2013 -0700
committer Glenn Kasten <gkasten@google.com> Fri Aug 16 14:10:13 2013 -0700
tree 6486a52a543beacdd79cfe3c5d40cb27692014de
parent 9e98235a9e9bb870e1c76911e3b4d00386a52c39

Fix races in reading activeTrack->mState without lock

Change-Id: I8cfbd9884fa5959a846ddb52cdc797266b896769

services/audioflinger/Threads.cpp

diff --git a/services/audioflinger/Threads.cpp b/services/audioflinger/Threads.cpp
index 44763a6..1564a44 100644
--- a/services/audioflinger/Threads.cpp
+++ b/services/audioflinger/Threads.cpp
@@ -4215,6 +4215,7 @@
     // start recording
     for (;;) {
         sp<RecordTrack> activeTrack;
+        TrackBase::track_state activeTrackState;
         Vector< sp<EffectChain> > effectChains;
 
         // sleep with mutex unlocked
@@ -4251,7 +4252,8 @@
                 continue;
             }
 
-            switch (activeTrack->mState) {
+            activeTrackState = activeTrack->mState;
+            switch (activeTrackState) {
             case TrackBase::PAUSING:
                 standby();
                 mActiveTrack.clear();
@@ -4284,16 +4286,15 @@
                 break;
 
             default:
-                LOG_FATAL("Unexpected activeTrack->mState %d", activeTrack->mState);
+                LOG_FATAL("Unexpected activeTrackState %d", activeTrackState);
             }
 
             lockEffectChains_l(effectChains);
         }
 
         // thread mutex is now unlocked, mActiveTrack unknown, activeTrack != 0, kept, immutable
-        // FIXME RecordThread::stop assigns to mState under lock, but we read without lock
-        if (activeTrack->mState != TrackBase::ACTIVE &&
-            activeTrack->mState != TrackBase::RESUMING) {
+        // activeTrack->mState unknown, activeTrackState immutable
+        if (activeTrackState != TrackBase::ACTIVE && activeTrackState != TrackBase::RESUMING) {
             unlockEffectChains(effectChains);
             doSleep = true;
             continue;
@@ -4345,8 +4346,8 @@
                         mBytesRead = mInput->stream->read(mInput->stream, readInto,
                                 mBufferSize);
                         if (mBytesRead <= 0) {
-                            // FIXME read mState without lock
-                            if ((mBytesRead < 0) && (activeTrack->mState == TrackBase::ACTIVE))
+                            // TODO: verify that it's benign to use a stale track state
+                            if ((mBytesRead < 0) && (activeTrackState == TrackBase::ACTIVE))
                             {
                                 ALOGE("Error reading audio input");
                                 // Force input into standby so that it tries to