Bug 1004669 - Fix leaks in MediaTaskQueue::Dispatch(). r=cpearce, a=sledru
authorJW Wang <jwwang@mozilla.com>
Tue, 13 May 2014 02:20:00 -0400
changeset 192263 662c516a1858
parent 192262 956578072396
child 192264 b334e79cf04f
push id3551
push userryanvm@gmail.com
push date2014-05-13 14:28 +0000
treeherdermozilla-beta@79e75d908e1d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscpearce, sledru
bugs1004669
milestone30.0
Bug 1004669 - Fix leaks in MediaTaskQueue::Dispatch(). r=cpearce, a=sledru
content/media/MediaDecoderStateMachine.cpp
content/media/MediaTaskQueue.cpp
content/media/MediaTaskQueue.h
--- a/content/media/MediaDecoderStateMachine.cpp
+++ b/content/media/MediaDecoderStateMachine.cpp
@@ -221,25 +221,19 @@ MediaDecoderStateMachine::MediaDecoderSt
 
 MediaDecoderStateMachine::~MediaDecoderStateMachine()
 {
   MOZ_ASSERT(NS_IsMainThread(), "Should be on main thread.");
   MOZ_COUNT_DTOR(MediaDecoderStateMachine);
   NS_ASSERTION(!mPendingWakeDecoder.get(),
                "WakeDecoder should have been revoked already");
 
-  if (mDecodeTaskQueue) {
-    mDecodeTaskQueue->Shutdown();
-    mDecodeTaskQueue = nullptr;
-  }
-
-  if (mTimer) {
-    mTimer->Cancel();
-  }
-  mTimer = nullptr;
+  MOZ_ASSERT(!mDecodeTaskQueue, "Should be released in SHUTDOWN");
+  // No need to cancel the timer here for we've done that in SHUTDOWN.
+  MOZ_ASSERT(!mTimer, "Should be released in SHUTDOWN");
   mReader = nullptr;
 
 #ifdef XP_WIN
   timeEndPeriod(1);
 #endif
 }
 
 bool MediaDecoderStateMachine::HasFutureAudio() const {
@@ -1553,23 +1547,23 @@ MediaDecoderStateMachine::DispatchDecode
   if (needToDecodeVideo) {
     EnsureVideoDecodeTaskQueued();
   }
 
   if (mIsReaderIdle == needIdle) {
     return;
   }
   mIsReaderIdle = needIdle;
-  nsRefPtr<nsIRunnable> event;
+  RefPtr<nsIRunnable> event;
   if (mIsReaderIdle) {
     event = NS_NewRunnableMethod(this, &MediaDecoderStateMachine::SetReaderIdle);
   } else {
     event = NS_NewRunnableMethod(this, &MediaDecoderStateMachine::SetReaderActive);
   }
-  if (NS_FAILED(mDecodeTaskQueue->Dispatch(event)) &&
+  if (NS_FAILED(mDecodeTaskQueue->Dispatch(event.forget())) &&
       mState != DECODER_STATE_SHUTDOWN) {
     NS_WARNING("Failed to dispatch event to set decoder idle state");
   }
 }
 
 nsresult
 MediaDecoderStateMachine::EnqueueDecodeSeekTask()
 {
@@ -2109,16 +2103,17 @@ nsresult MediaDecoderStateMachine::RunSt
       // thread pools alive. So break it here.
       mReader->AudioQueue().ClearListeners();
       mReader->VideoQueue().ClearListeners();
 
       {
         ReentrantMonitorAutoExit exitMon(mDecoder->GetReentrantMonitor());
         // Wait for the thread decoding to exit.
         mDecodeTaskQueue->Shutdown();
+        mDecodeTaskQueue = nullptr;
         mReader->ReleaseMediaResources();
       }
       // Now that those threads are stopped, there's no possibility of
       // mPendingWakeDecoder being needed again. Revoke it.
       mPendingWakeDecoder = nullptr;
 
       MOZ_ASSERT(mState == DECODER_STATE_SHUTDOWN,
                  "How did we escape from the shutdown state?");
@@ -2132,16 +2127,21 @@ nsresult MediaDecoderStateMachine::RunSt
       // event runner running this function (which holds a reference to the
       // state machine) needs to finish and be released in order to allow
       // that. So we dispatch an event to run after this event runner has
       // finished and released its monitor/references. That event then will
       // dispatch an event to the main thread to release the decoder and
       // state machine.
       GetStateMachineThread()->Dispatch(
         new nsDispatchDisposeEvent(mDecoder, this), NS_DISPATCH_NORMAL);
+
+      if (mTimer) {
+        mTimer->Cancel();
+        mTimer = nullptr;
+      }
       return NS_OK;
     }
 
     case DECODER_STATE_DORMANT: {
       if (IsPlaying()) {
         StopPlayback();
       }
       StopAudioThread();
--- a/content/media/MediaTaskQueue.cpp
+++ b/content/media/MediaTaskQueue.cpp
@@ -22,17 +22,17 @@ MediaTaskQueue::MediaTaskQueue(Temporary
 MediaTaskQueue::~MediaTaskQueue()
 {
   MonitorAutoLock mon(mQueueMonitor);
   MOZ_ASSERT(mIsShutdown);
   MOZ_COUNT_DTOR(MediaTaskQueue);
 }
 
 nsresult
-MediaTaskQueue::Dispatch(nsIRunnable* aRunnable)
+MediaTaskQueue::Dispatch(TemporaryRef<nsIRunnable> aRunnable)
 {
   MonitorAutoLock mon(mQueueMonitor);
   if (mIsShutdown) {
     return NS_ERROR_FAILURE;
   }
   mTasks.push(aRunnable);
   if (mIsRunning) {
     return NS_OK;
--- a/content/media/MediaTaskQueue.h
+++ b/content/media/MediaTaskQueue.h
@@ -24,17 +24,17 @@ class SharedThreadPool;
 // They may be executed on different threads, and a memory barrier is used
 // to make this threadsafe for objects that aren't already threadsafe.
 class MediaTaskQueue : public AtomicRefCounted<MediaTaskQueue> {
 public:
   MOZ_DECLARE_REFCOUNTED_TYPENAME(MediaTaskQueue)
   MediaTaskQueue(TemporaryRef<SharedThreadPool> aPool);
   ~MediaTaskQueue();
 
-  nsresult Dispatch(nsIRunnable* aRunnable);
+  nsresult Dispatch(TemporaryRef<nsIRunnable> aRunnable);
 
   // Removes all pending tasks from the task queue, and blocks until
   // the currently running task (if any) finishes.
   void Flush();
 
   // Blocks until all tasks finish executing, then shuts down the task queue
   // and exits.
   void Shutdown();