Bug 1365534 - Close temp file in FileBlockCache thread - r=cpearce

Closing the temporary file was done in the FileBlockCache destructor, which is
called on the main thread.

Instead, we are now dispatching a new final task from Close, which will close
the temporary file and then shutdown the thread.
The thread and FD ownerships are transferred to that final task, so that no
more tasks may be unexpectly queued to that thread, and both members can
immediately be reused after Close() returns.

MozReview-Commit-ID: L9djYGhDFn4

--HG--
extra : rebase_source : bedad374454ec94966d10552b8aea26c8997fbb9

dom/media/FileBlockCache.cpp

@@ -103,43 +103,53 @@ FileBlockCache::FileBlockCache()
 FileBlockCache::~FileBlockCache()
 {
   NS_ASSERTION(!mIsOpen, "Should Close() FileBlockCache before destroying");
-  {
-    // Note, mThread will be shutdown by the time this runs, so we won't
-    // block while taking mFileMonitor.
-    MonitorAutoLock mon(mFileMonitor);
-    if (mFD) {
-      PRStatus prrc;
-      prrc = PR_Close(mFD);
-      if (prrc != PR_SUCCESS) {
-        NS_WARNING("PR_Close() failed.");
-      }
-      mFD = nullptr;
-    }
-  }
 }
 
 void FileBlockCache::Close()
 {
   LOG("Close()");
 
-  MonitorAutoLock mon(mDataMonitor);
-  if (!mIsOpen) {
-    return;
+  nsCOMPtr<nsIThread> thread;
+  {
+    MonitorAutoLock mon(mDataMonitor);
+    if (!mIsOpen) {
+      return;
+    }
+    mIsOpen = false;
+    if (!mThread) {
+      return;
+    }
+    thread.swap(mThread);
   }
-  mIsOpen = false;
-  if (!mThread) {
-    return;
+
+  PRFileDesc* fd;
+  {
+    MonitorAutoLock lock(mFileMonitor);
+    fd = mFD;
+    mFD = nullptr;
   }
-  // We must shut down the thread in another runnable. This is called
-  // while we're shutting down the media cache, and nsIThread::Shutdown()
-  // can cause events to run before it completes, which could end up
-  // opening more streams, while the media cache is shutting down and
-  // releasing memory etc! Also note we close mFD in the destructor so
-  // as to not disturb any IO that's currently running.
-  nsCOMPtr<nsIRunnable> event = new ShutdownThreadEvent(mThread);
-  SystemGroup::Dispatch(
-    "ShutdownThreadEvent", TaskCategory::Other, event.forget());
-  mThread = nullptr;
+
+  // Let the thread close the FD, and then trigger its own shutdown.
+  // Note that mThread is now empty, so no other task will be posted there.
+  // Also mThread and mFD are empty and therefore can be reused immediately.
+  nsresult rv = thread->Dispatch(NS_NewRunnableFunction([thread, fd] {
+    if (fd) {
+      PRStatus prrc;
+      prrc = PR_Close(fd);
+      if (prrc != PR_SUCCESS) {
+        NS_WARNING("PR_Close() failed.");
+      }
+    }
+    // We must shut down the thread in another runnable. This is called
+    // while we're shutting down the media cache, and nsIThread::Shutdown()
+    // can cause events to run before it completes, which could end up
+    // opening more streams, while the media cache is shutting down and
+    // releasing memory etc!
+    nsCOMPtr<nsIRunnable> event = new ShutdownThreadEvent(thread);
+    SystemGroup::Dispatch(
+      "ShutdownThreadEvent", TaskCategory::Other, event.forget());
+  }), NS_DISPATCH_NORMAL);
+  NS_ENSURE_SUCCESS_VOID(rv);
 }
 
 template<typename Container, typename Value>