Bug 774597. Avoid accessing MediaStreamGraphImpl members after the graph object may have been cleaned up by the main thread. r=jesup
authorRobert O'Callahan <robert@ocallahan.org>
Wed, 18 Jul 2012 01:02:06 -0400
changeset 99628 58525c0d69f24a7b1bfd59f97efb33c9466bab05
parent 99627 cedfa313d168433bf943734078506c811a22cb84
child 99629 a3ee29cb995c46f2c9f64db2c53767aa1bbbed85
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersjesup
bugs774597
milestone17.0a1
Bug 774597. Avoid accessing MediaStreamGraphImpl members after the graph object may have been cleaned up by the main thread. r=jesup
content/media/MediaStreamGraph.cpp
--- a/content/media/MediaStreamGraph.cpp
+++ b/content/media/MediaStreamGraph.cpp
@@ -1320,31 +1320,37 @@ MediaStreamGraphImpl::RunThread()
         allBlockedForever = false;
       }
     }
     if (!allBlockedForever || audioStreamsActive > 0) {
       EnsureNextIteration();
     }
 
     {
-      MonitorAutoLock lock(mMonitor);
+      // Not using MonitorAutoLock since we need to unlock in a way
+      // that doesn't match lexical scopes.
+      mMonitor.Lock();
       PrepareUpdatesToMainThreadState();
       if (mForceShutDown || (IsEmpty() && mMessageQueue.IsEmpty())) {
         // Enter shutdown mode. The stable-state handler will detect this
         // and complete shutdown. Destroy any streams immediately.
         LOG(PR_LOG_DEBUG, ("MediaStreamGraph %p waiting for main thread cleanup", this));
+        // Commit to shutting down this graph object.
         mLifecycleState = LIFECYCLE_WAITING_FOR_MAIN_THREAD_CLEANUP;
-        {
-          MonitorAutoUnlock unlock(mMonitor);
-          // Unlock mMonitor while destroying our streams, since
-          // SourceMediaStream::DestroyImpl needs to take its lock while
-          // we're not holding mMonitor.
-          for (PRUint32 i = 0; i < mStreams.Length(); ++i) {
-            mStreams[i]->DestroyImpl();
-          }
+        // Move mStreams to a temporary array, because after we unlock
+        // mMonitor, 'this' may be deleted by the main thread.
+        nsTArray<nsRefPtr<MediaStream> > streams;
+        mStreams.SwapElements(streams);
+
+        mMonitor.Unlock();
+        // Unlock mMonitor while destroying our streams, since
+        // SourceMediaStream::DestroyImpl needs to take its lock while
+        // we're not holding mMonitor.
+        for (PRUint32 i = 0; i < streams.Length(); ++i) {
+          streams[i]->DestroyImpl();
         }
         return;
       }
 
       PRIntervalTime timeout = PR_INTERVAL_NO_TIMEOUT;
       TimeStamp now = TimeStamp::Now();
       if (mNeedAnotherIteration) {
         PRInt64 timeoutMS = MEDIA_GRAPH_TARGET_PERIOD_MS -
@@ -1355,24 +1361,26 @@ MediaStreamGraphImpl::RunThread()
         timeout = PR_MillisecondsToInterval(PRUint32(timeoutMS));
         LOG(PR_LOG_DEBUG, ("Waiting for next iteration; at %f, timeout=%f",
                            (now - mInitialTimeStamp).ToSeconds(), timeoutMS/1000.0));
         mWaitState = WAITSTATE_WAITING_FOR_NEXT_ITERATION;
       } else {
         mWaitState = WAITSTATE_WAITING_INDEFINITELY;
       }
       if (timeout > 0) {
-        lock.Wait(timeout);
+        mMonitor.Wait(timeout);
         LOG(PR_LOG_DEBUG, ("Resuming after timeout; at %f, elapsed=%f",
                            (TimeStamp::Now() - mInitialTimeStamp).ToSeconds(),
                            (TimeStamp::Now() - now).ToSeconds()));
       }
       mWaitState = WAITSTATE_RUNNING;
       mNeedAnotherIteration = false;
       messageQueue.SwapElements(mMessageQueue);
+
+      mMonitor.Unlock();
     }
   }
 }
 
 void
 MediaStreamGraphImpl::ApplyStreamUpdate(StreamUpdate* aUpdate)
 {
   mMonitor.AssertCurrentThreadOwns();