ToneGenerator: fix stop/destroy concurrency

There is a problem if the stopTone() method is called
from two different threads (for instance if the destructor is called
while stopTone() is waiting for the audio callback to finish).

In this case, the second call to stopTone() will not wait for the
condition to be signaled and call clearWaveGens() while the callback
can still be active, thus causing a crash.

There is a similar problem in case of concurrent calls to startTone()
and stopTone().

The fix consists in making sure that stopTone() always waits for call
back completion or timeout and exits before calling clearWaveGens()
if a concurrent start request is detected.

Bug 8163071

Change-Id: I9ddb4390407701dcad5bf83660fd9903f0d72268

diff --git a/media/libmedia/ToneGenerator.cpp b/media/libmedia/ToneGenerator.cpp
index 58d495e..3554608 100644 (file)
--- a/media/libmedia/ToneGenerator.cpp
+++ b/media/libmedia/ToneGenerator.cpp
@@ -976,21 +976,26 @@ void ToneGenerator::stopTone() {
     ALOGV("stopTone");
 
     mLock.lock();
-    if (mState == TONE_PLAYING || mState == TONE_STARTING || mState == TONE_RESTARTING) {
-        mState = TONE_STOPPING;
+    if (mState != TONE_IDLE && mState != TONE_INIT) {
+        if (mState == TONE_PLAYING || mState == TONE_STARTING || mState == TONE_RESTARTING) {
+            mState = TONE_STOPPING;
+        }
         ALOGV("waiting cond");
         status_t lStatus = mWaitCbkCond.waitRelative(mLock, seconds(3));
         if (lStatus == NO_ERROR) {
+            // If the tone was restarted exit now before calling clearWaveGens();
+            if (mState != TONE_INIT) {
+                return;
+            }
             ALOGV("track stop complete, time %d", (unsigned int)(systemTime()/1000000));
         } else {
             ALOGE("--- Stop timed out");
             mState = TONE_IDLE;
             mpAudioTrack->stop();
         }
+        clearWaveGens();
     }
 
-    clearWaveGens();
-
     mLock.unlock();
 }
 
@@ -1299,7 +1304,7 @@ audioCallback_EndLoop:
         }
 
         if (lSignal)
-            lpToneGen->mWaitCbkCond.signal();
+            lpToneGen->mWaitCbkCond.broadcast();
         lpToneGen->mLock.unlock();
     }
 }