Bug 1029372 - Clear AudioQueue only after shutting down audio thread to avoid race in accessing AudioQueue. r=kinetik, a=lmandel
authorJW Wang <jwwang@mozilla.com>
Sun, 21 Sep 2014 23:41:00 -0400
changeset 225005 2e7c9193980f94ea4f1b7028af19da552966aa1d
parent 225004 33c183bacd646dc774a71b93daf50186aac097c3
child 225006 4ae055d7bc847dd8554b43aa678e3d77a3f3be8c
push id3979
push userraliiev@mozilla.com
push dateMon, 13 Oct 2014 16:35:44 +0000
treeherdermozilla-beta@30f2cc610691 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskinetik, lmandel
bugs1029372
milestone34.0a2
Bug 1029372 - Clear AudioQueue only after shutting down audio thread to avoid race in accessing AudioQueue. r=kinetik, a=lmandel
content/media/MediaDecoderStateMachine.cpp
--- a/content/media/MediaDecoderStateMachine.cpp
+++ b/content/media/MediaDecoderStateMachine.cpp
@@ -1448,16 +1448,22 @@ void MediaDecoderStateMachine::Play()
   ScheduleStateMachine();
 }
 
 void MediaDecoderStateMachine::ResetPlayback()
 {
   MOZ_ASSERT(mState == DECODER_STATE_SEEKING ||
              mState == DECODER_STATE_SHUTDOWN ||
              mState == DECODER_STATE_DORMANT);
+
+  // Audio thread should've been stopped at the moment. Otherwise, AudioSink
+  // might be accessing AudioQueue outside of the decoder monitor while we
+  // are clearing the queue and causes crash for no samples to be popped.
+  MOZ_ASSERT(!mAudioSink);
+
   mVideoFrameEndTime = -1;
   mAudioStartTime = -1;
   mAudioEndTime = -1;
   mAudioCompleted = false;
   AudioQueue().Reset();
   VideoQueue().Reset();
   mFirstVideoFrameAfterSeek = nullptr;
   mDropAudioUntilNextDiscontinuity = true;
@@ -1533,34 +1539,40 @@ void MediaDecoderStateMachine::Seek(cons
 
 void MediaDecoderStateMachine::StopAudioThread()
 {
   NS_ASSERTION(OnDecodeThread() ||
                OnStateMachineThread(), "Should be on decode thread or state machine thread");
   AssertCurrentThreadInMonitor();
 
   if (mStopAudioThread) {
-    // Nothing to do, since the thread is already stopping
+    // Audio sink is being stopped in another thread. Wait until finished.
+    while (mAudioSink) {
+      mDecoder->GetReentrantMonitor().Wait();
+    }
     return;
   }
 
   mStopAudioThread = true;
+  // Wake up audio sink so that it can reach the finish line.
   mDecoder->GetReentrantMonitor().NotifyAll();
   if (mAudioSink) {
     DECODER_LOG("Shutdown audio thread");
     mAudioSink->PrepareToShutdown();
     {
       ReentrantMonitorAutoExit exitMon(mDecoder->GetReentrantMonitor());
       mAudioSink->Shutdown();
     }
     mAudioSink = nullptr;
     // Now that the audio sink is dead, try sending data to our MediaStream(s).
     // That may have been waiting for the audio thread to stop.
     SendStreamData();
   }
+  // Wake up those waiting for audio sink to finish.
+  mDecoder->GetReentrantMonitor().NotifyAll();
 }
 
 nsresult
 MediaDecoderStateMachine::EnqueueDecodeMetadataTask()
 {
   AssertCurrentThreadInMonitor();
   MOZ_ASSERT(mState == DECODER_STATE_DECODING_METADATA);
 
@@ -2267,33 +2279,24 @@ nsresult MediaDecoderStateMachine::RunSt
   NS_ENSURE_TRUE(resource, NS_ERROR_NULL_POINTER);
 
   switch (mState) {
     case DECODER_STATE_SHUTDOWN: {
       if (IsPlaying()) {
         StopPlayback();
       }
 
+      StopAudioThread();
       FlushDecoding();
 
       // Put a task in the decode queue to shutdown the reader.
       RefPtr<nsIRunnable> task;
       task = NS_NewRunnableMethod(mReader, &MediaDecoderReader::Shutdown);
       mDecodeTaskQueue->Dispatch(task);
 
-      StopAudioThread();
-      // If mAudioSink is non-null after StopAudioThread completes, we are
-      // running in a nested event loop waiting for Shutdown() on
-      // mAudioSink to complete.  Return to the event loop and let it
-      // finish processing before continuing with shutdown.
-      if (mAudioSink) {
-        MOZ_ASSERT(mStopAudioThread);
-        return NS_OK;
-      }
-
       {
         // Wait for the thread decoding to exit.
         ReentrantMonitorAutoExit exitMon(mDecoder->GetReentrantMonitor());
         mDecodeTaskQueue->Shutdown();
         mDecodeTaskQueue = nullptr;
       }
 
       // The reader's listeners hold references to the state machine,
@@ -2327,18 +2330,18 @@ nsresult MediaDecoderStateMachine::RunSt
       DECODER_LOG("SHUTDOWN OK");
       return NS_OK;
     }
 
     case DECODER_STATE_DORMANT: {
       if (IsPlaying()) {
         StopPlayback();
       }
+      StopAudioThread();
       FlushDecoding();
-      StopAudioThread();
       // Now that those threads are stopped, there's no possibility of
       // mPendingWakeDecoder being needed again. Revoke it.
       mPendingWakeDecoder = nullptr;
       {
         ReentrantMonitorAutoExit exitMon(mDecoder->GetReentrantMonitor());
         // Wait for the thread decoding, if any, to exit.
         mDecodeTaskQueue->AwaitIdle();
         mReader->ReleaseMediaResources();