commit 1d491ff06f4b9c90ff24fe953b90d0843eaf1c04
author Glenn Kasten <gkasten@google.com> Mon Jul 16 14:28:13 2012 -0700
committer Glenn Kasten <gkasten@google.com> Wed Jul 25 14:41:03 2012 -0700
tree 006faa29a2ad0ca2ea18c8e876c43749d72d138d
parent bf5bea96f236adb5eef78c2f414ef82b3602a0f7

Fix races in AudioRecord stop()

Change-Id: Id0ac1915f57fef4a938c7f90989c1162a8b6c51c

services/audioflinger/AudioFlinger.cpp

diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp
index 5e29f3b..07fa791 100644
--- a/services/audioflinger/AudioFlinger.cpp
+++ b/services/audioflinger/AudioFlinger.cpp
@@ -5362,11 +5362,18 @@
     sp<ThreadBase> thread = mThread.promote();
     if (thread != 0) {
         RecordThread *recordThread = (RecordThread *)thread.get();
-        recordThread->stop(this);
-        TrackBase::reset();
-        // Force overrun condition to avoid false overrun callback until first data is
-        // read from buffer
-        android_atomic_or(CBLK_UNDERRUN_ON, &mCblk->flags);
+        recordThread->mLock.lock();
+        bool doStop = recordThread->stop_l(this);
+        if (doStop) {
+            TrackBase::reset();
+            // Force overrun condition to avoid false overrun callback until first data is
+            // read from buffer
+            android_atomic_or(CBLK_UNDERRUN_ON, &mCblk->flags);
+        }
+        recordThread->mLock.unlock();
+        if (doStop) {
+            AudioSystem::stopInput(recordThread->id());
+        }
     }
 }
 
@@ -6278,27 +6285,23 @@
     }
 }
 
-void AudioFlinger::RecordThread::stop(RecordThread::RecordTrack* recordTrack) {
+bool AudioFlinger::RecordThread::stop_l(RecordThread::RecordTrack* recordTrack) {
     ALOGV("RecordThread::stop");
-    sp<ThreadBase> strongMe = this;
-    {
-        AutoMutex lock(mLock);
-        if (mActiveTrack != 0 && recordTrack == mActiveTrack.get()) {
-            mActiveTrack->mState = TrackBase::PAUSING;
-            // do not wait for mStartStopCond if exiting
-            if (exitPending()) {
-                return;
-            }
-            mStartStopCond.wait(mLock);
-            // if we have been restarted, recordTrack == mActiveTrack.get() here
-            if (exitPending() || mActiveTrack == 0 || recordTrack != mActiveTrack.get()) {
-                mLock.unlock();
-                AudioSystem::stopInput(mId);
-                mLock.lock();
-                ALOGV("Record stopped OK");
-            }
-        }
+    if (recordTrack != mActiveTrack.get() || recordTrack->mState == TrackBase::PAUSING) {
+        return false;
     }
+    recordTrack->mState = TrackBase::PAUSING;
+    // do not wait for mStartStopCond if exiting
+    if (exitPending()) {
+        return true;
+    }
+    mStartStopCond.wait(mLock);
+    // if we have been restarted, recordTrack == mActiveTrack.get() here
+    if (exitPending() || recordTrack != mActiveTrack.get()) {
+        ALOGV("Record stopped OK");
+        return true;
+    }
+    return false;
 }
 
 bool AudioFlinger::RecordThread::isValidSyncEvent(const sp<SyncEvent>& event)

services/audioflinger/AudioFlinger.h

diff --git a/services/audioflinger/AudioFlinger.h b/services/audioflinger/AudioFlinger.h
index c9a3c3f..2945417 100644
--- a/services/audioflinger/AudioFlinger.h
+++ b/services/audioflinger/AudioFlinger.h
@@ -1432,7 +1432,11 @@
                 status_t    start(RecordTrack* recordTrack,
                                   AudioSystem::sync_event_t event,
                                   int triggerSession);
-                void        stop(RecordTrack* recordTrack);
+
+                // ask the thread to stop the specified track, and
+                // return true if the caller should then do it's part of the stopping process
+                bool        stop_l(RecordTrack* recordTrack);
+
                 void        dump(int fd, const Vector<String16>& args);
                 AudioStreamIn* clearInput();
                 virtual audio_stream_t* stream() const;