commit 735f45fbc37d7905ffb722f40727edbed82319b7
author Glenn Kasten <gkasten@google.com> Mon Aug 18 15:51:59 2014 -0700
committer Glenn Kasten <gkasten@google.com> Mon Aug 18 16:19:01 2014 -0700
tree e8abf5e80a5b9a05efeb9f8ea1f3ace03ffea6a4
parent a7ee3549823af3dc4b22284f1a2fe4fbf31cf30b

Fix memory access error due to race during fast track removal

Bug: 16205169
Change-Id: Ie15ec2a737b6ddd16b2d26030d01564ff70177f5

services/audioflinger/Threads.cpp

diff --git a/services/audioflinger/Threads.cpp b/services/audioflinger/Threads.cpp
index 193f8e4..f721d5c 100644
--- a/services/audioflinger/Threads.cpp
+++ b/services/audioflinger/Threads.cpp
@@ -5017,9 +5017,12 @@
         // activeTracks accumulates a copy of a subset of mActiveTracks
         Vector< sp<RecordTrack> > activeTracks;
 
-        // reference to the (first and only) fast track
+        // reference to the (first and only) active fast track
         sp<RecordTrack> fastTrack;
 
+        // reference to a fast track which is about to be removed
+        sp<RecordTrack> fastTrackToRemove;
+
         { // scope for mLock
             Mutex::Autolock _l(mLock);
 
@@ -5058,6 +5061,10 @@
 
                 activeTrack = mActiveTracks[i];
                 if (activeTrack->isTerminated()) {
+                    if (activeTrack->isFastTrack()) {
+                        ALOG_ASSERT(fastTrackToRemove == 0);
+                        fastTrackToRemove = activeTrack;
+                    }
                     removeTrack_l(activeTrack);
                     mActiveTracks.remove(activeTrack);
                     mActiveTracksGen++;
@@ -5130,10 +5137,12 @@
             effectChains[i]->process_l();
         }
 
-        // Start the fast capture if it's not already running
+        // Push a new fast capture state if fast capture is not already running, or cblk change
         if (mFastCapture != 0) {
             FastCaptureStateQueue *sq = mFastCapture->sq();
             FastCaptureState *state = sq->begin();
+            bool didModify = false;
+            FastCaptureStateQueue::block_t block = FastCaptureStateQueue::BLOCK_UNTIL_PUSHED;
             if (state->mCommand != FastCaptureState::READ_WRITE /* FIXME &&
                     (kUseFastMixer != FastMixer_Dynamic || state->mTrackMask > 1)*/) {
                 if (state->mCommand == FastCaptureState::COLD_IDLE) {
@@ -5147,19 +5156,32 @@
                 mFastCaptureDumpState.increaseSamplingN(mAudioFlinger->isLowRamDevice() ?
                         FastCaptureDumpState::kSamplingNforLowRamDevice : FastMixerDumpState::kSamplingN);
 #endif
-                state->mCblk = fastTrack != 0 ? fastTrack->cblk() : NULL;
-                sq->end();
-                sq->push(FastCaptureStateQueue::BLOCK_UNTIL_PUSHED);
+                didModify = true;
+            }
+            audio_track_cblk_t *cblkOld = state->mCblk;
+            audio_track_cblk_t *cblkNew = fastTrack != 0 ? fastTrack->cblk() : NULL;
+            if (cblkNew != cblkOld) {
+                state->mCblk = cblkNew;
+                // block until acked if removing a fast track
+                if (cblkOld != NULL) {
+                    block = FastCaptureStateQueue::BLOCK_UNTIL_ACKED;
+                }
+                didModify = true;
+            }
+            sq->end(didModify);
+            if (didModify) {
+                sq->push(block);
 #if 0
                 if (kUseFastCapture == FastCapture_Dynamic) {
                     mNormalSource = mPipeSource;
                 }
 #endif
-            } else {
-                sq->end(false /*didModify*/);
             }
         }
 
+        // now run the fast track destructor with thread mutex unlocked
+        fastTrackToRemove.clear();
+
         // Read from HAL to keep up with fastest client if multiple active tracks, not slowest one.
         // Only the client(s) that are too slow will overrun. But if even the fastest client is too
         // slow, then this RecordThread will overrun by not calling HAL read often enough.