Fix issue 2025872: Deadlock in SoundPool.stop

There were 2 problems in SoundPool:

1 If not using the shared memory buffer mode, there was a problem when a sound channel was stolen. The new channel could sometimes not be added to the restart
list if the AudioTrack callback thread was stopped before the underrun callback was called.

The SoundChannel::play() method is modified so that SoundPool::done() is called immediately after stopping the channel. There is a possibility that done() is called a second time by the callback; in this case it will be added 2 times to the restart list but the second start request will be ignored as the first one will have reset the next chennel ID when processed.

2 There was a deadlock on SoundPool::mLock if SoundPool::stop() was called while a channel restart was pending:
 SoundPool::stop() lock mLock -> SoundChannel::stop() -> SoundPool::done() -> SoundPool::addToRestartList() -> try to lock mLock == deadlock

A second mutex mRestartLock is added to protect the restart list mRestart. mLock is still used to protect mChannels list but mRestart is now used to
protect access to mRestart by restart thread and client thread.

2 files changed, 9 insertions, 11 deletions

diff --git a/media/jni/soundpool/SoundPool.cpp b/media/jni/soundpool/SoundPool.cpp
index 1fdecdd054df..00a121bfc6e5 100644
--- a/media/jni/soundpool/SoundPool.cpp
+++ b/media/jni/soundpool/SoundPool.cpp
@@ -93,7 +93,7 @@ SoundPool::~SoundPool()
 
 void SoundPool::addToRestartList(SoundChannel* channel)
 {
-    Mutex::Autolock lock(&mLock);
+    Mutex::Autolock lock(&mRestartLock);
     mRestart.push_back(channel);
     mCondition.signal();
 }
@@ -106,9 +106,9 @@ int SoundPool::beginThread(void* arg)
 
 int SoundPool::run()
 {
-    mLock.lock();
+    mRestartLock.lock();
     while (!mQuit) {
-        mCondition.wait(mLock);
+        mCondition.wait(mRestartLock);
         LOGV("awake");
         if (mQuit) break;
 
@@ -125,19 +125,19 @@ int SoundPool::run()
 
     mRestart.clear();
     mCondition.signal();
-    mLock.unlock();
+    mRestartLock.unlock();
     LOGV("goodbye");
     return 0;
 }
 
 void SoundPool::quit()
 {
-    mLock.lock();
+    mRestartLock.lock();
     mQuit = true;
     mCondition.signal();
-    mCondition.wait(mLock);
+    mCondition.wait(mRestartLock);
     LOGV("return from quit");
-    mLock.unlock();
+    mRestartLock.unlock();
 }
 
 bool SoundPool::startThreads()
@@ -484,11 +484,8 @@ void SoundChannel::play(const sp<Sample>& sample, int nextChannelID, float leftV
     // if not idle, this voice is being stolen
     if (mState != IDLE) {
         LOGV("channel %d stolen - event queued for channel %d", channelID(), nextChannelID);
-        stop_l();
         mNextEvent.set(sample, nextChannelID, leftVolume, rightVolume, priority, loop, rate);
-#ifdef USE_SHARED_MEM_BUFFER
-        mSoundPool->done(this);
-#endif
+        stop();
         return;
     }
 
diff --git a/media/jni/soundpool/SoundPool.h b/media/jni/soundpool/SoundPool.h
index 78027812fb95..ab86e90ef3a7 100644
--- a/media/jni/soundpool/SoundPool.h
+++ b/media/jni/soundpool/SoundPool.h
@@ -204,6 +204,7 @@ private:
 
     jobject                 mSoundPoolRef;
     Mutex                   mLock;
+    Mutex                   mRestartLock;
     Condition               mCondition;
     SoundPoolThread*        mDecodeThread;
     SoundChannel*           mChannelPool;