Bug 1349746 - Ensure we close media cache's temporary file if the cache shuts down while we're waiting for the FD to arrive. r?jwwang draft
authorChris Pearce <cpearce@mozilla.com>
Fri, 07 Apr 2017 17:51:47 +1200
changeset 557746 1d1c41a42f9c4590e81e2fe59d21d8faa8b6b831
parent 557485 fdd53f7d454d15a0b0d82d6bdae2c4fed837d048
child 623140 721ae5ac12323155c8de3356c9f88009b9f08b66
push id52810
push userbmo:cpearce@mozilla.com
push dateFri, 07 Apr 2017 08:25:43 +0000
reviewersjwwang
bugs1349746
milestone55.0a1
Bug 1349746 - Ensure we close media cache's temporary file if the cache shuts down while we're waiting for the FD to arrive. r?jwwang MozReview-Commit-ID: HNXt3pYb1Iv
dom/media/FileBlockCache.cpp
dom/media/FileBlockCache.h
--- a/dom/media/FileBlockCache.cpp
+++ b/dom/media/FileBlockCache.cpp
@@ -22,25 +22,37 @@ void
 FileBlockCache::SetCacheFile(PRFileDesc* aFD)
 {
   MOZ_ASSERT(NS_IsMainThread());
   FBC_LOG(LogLevel::Debug,
           ("FileBlockCache::SetFD(aFD=%p) mIsOpen=%d", aFD, mIsOpen));
 
   if (!aFD) {
     // Failed to get a temporary file. Shutdown.
-    mInitPromise->Reject(NS_ERROR_FAILURE, __func__);
     Close();
     return;
   }
   {
     MonitorAutoLock lock(mFileMonitor);
     mFD = aFD;
   }
-  mInitPromise->Resolve(true, __func__);
+  {
+    MonitorAutoLock lock(mDataMonitor);
+    if (!mIsOpen) {
+      // We've been closed while waiting for the file descriptor. Bail out.
+      // Rely on the destructor to close the file descriptor.
+      return;
+    }
+    mInitialized = true;
+    if (mIsWriteScheduled) {
+      // A write was scheduled while waiting for FD. We need to dispatch a
+      // task to service the request.
+      mThread->Dispatch(this, NS_DISPATCH_NORMAL);
+    }
+  }
 }
 
 nsresult
 FileBlockCache::Init()
 {
   FBC_LOG(LogLevel::Debug, ("FileBlockCache::Init()"));
 
   MOZ_ASSERT(NS_IsMainThread());
@@ -48,24 +60,22 @@ FileBlockCache::Init()
   MonitorAutoLock mon(mDataMonitor);
   nsresult rv = NS_NewNamedThread("FileBlockCache",
                                   getter_AddRefs(mThread),
                                   nullptr,
                                   SharedThreadPool::kStackSize);
   if (NS_FAILED(rv)) {
     return rv;
   }
-  mAbstractThread = AbstractThread::CreateXPCOMThreadWrapper(mThread, false);
   mIsOpen = true;
 
-  mInitPromise = new GenericPromise::Private(__func__);
   if (XRE_IsParentProcess()) {
     rv = NS_OpenAnonymousTemporaryFile(&mFD);
     if (NS_SUCCEEDED(rv)) {
-      mInitPromise->Resolve(true, __func__);
+      mInitialized = true;
     }
   } else {
     // We must request a temporary file descriptor from the parent process.
     RefPtr<FileBlockCache> self = this;
     rv = dom::ContentChild::GetSingleton()->AsyncOpenAnonymousTemporaryFile(
       [self](PRFileDesc* aFD) { self->SetCacheFile(aFD); });
   }
 
@@ -106,21 +116,23 @@ FileBlockCache::~FileBlockCache()
 
 void FileBlockCache::Close()
 {
   FBC_LOG(LogLevel::Debug, ("FileBlockCache::Close"));
 
   NS_ASSERTION(NS_IsMainThread(), "Only call on main thread");
 
   MonitorAutoLock mon(mDataMonitor);
+  if (!mIsOpen) {
+    return;
+  }
   mIsOpen = false;
   if (!mThread) {
     return;
   }
-  mAbstractThread = 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(
@@ -168,24 +180,22 @@ void FileBlockCache::EnsureWriteSchedule
 {
   mDataMonitor.AssertCurrentThreadOwns();
   MOZ_ASSERT(mIsOpen);
 
   if (mIsWriteScheduled) {
     return;
   }
   mIsWriteScheduled = true;
-
-  RefPtr<FileBlockCache> self = this;
-  mInitPromise->Then(mAbstractThread,
-                     __func__,
-                     [self](bool aValue) { self->Run(); },
-                     [self](nsresult rv) {}
-                     // Failure handled by EnsureInitialized.
-                     );
+  if (!mInitialized) {
+    // We're still waiting on a file descriptor. When it arrives,
+    // the write will be scheduled.
+    return;
+  }
+  mThread->Dispatch(this, NS_DISPATCH_NORMAL);
 }
 
 nsresult FileBlockCache::Seek(int64_t aOffset)
 {
   mFileMonitor.AssertCurrentThreadOwns();
 
   if (mFDCurrentPos != aOffset) {
     MOZ_ASSERT(mFD);
--- a/dom/media/FileBlockCache.h
+++ b/dom/media/FileBlockCache.h
@@ -158,39 +158,35 @@ private:
   // and mFDCurrentPos. Don't hold mDataMonitor while holding mFileMonitor!
   // mDataMonitor must be owned while accessing any of the following data
   // fields or methods.
   Monitor mDataMonitor;
   // Ensures we either are running the event to preform IO, or an event
   // has been dispatched to preform the IO.
   // mDataMonitor must be owned while calling this.
   void EnsureWriteScheduled();
-  // Promise that tracks the request for an anonymous temporary file for the
-  // cache to store data into. The file descriptor must be requested from the
-  // parent process when the cache is initialized. While this promise is
-  // outstanding, the FileBlockCache buffers blocks in memory, and reads
-  // against the cache are serviced from the in-memory buffers.
-  RefPtr<GenericPromise::Private> mInitPromise;
 
   // Array of block changes to made. If mBlockChanges[offset/BLOCK_SIZE] == nullptr,
   // then the block has no pending changes to be written, but if
   // mBlockChanges[offset/BLOCK_SIZE] != nullptr, then either there's a block
   // cached in memory waiting to be written, or this block is the target of a
   // block move.
   nsTArray< RefPtr<BlockChange> > mBlockChanges;
   // Thread upon which block writes and block moves are performed. This is
   // created upon open, and shutdown (asynchronously) upon close (on the
   // main thread).
   nsCOMPtr<nsIThread> mThread;
-  // Wrapper for mThread.
-  RefPtr<AbstractThread> mAbstractThread;
   // Queue of pending block indexes that need to be written or moved.
   std::deque<int32_t> mChangeIndexList;
   // True if we've dispatched an event to commit all pending block changes
   // to file on mThread.
   bool mIsWriteScheduled;
-  // True if the writer is ready to write data to file.
+  // True if the writer is ready to enqueue writes.
   bool mIsOpen;
+  // True if we've got a temporary file descriptor. Note: we don't use mFD
+  // directly as that's synchronized via mFileMonitor and we need to make
+  // decisions about whether we can write while holding mDataMonitor.
+  bool mInitialized = false;
 };
 
 } // End namespace mozilla.
 
 #endif /* FILE_BLOCK_CACHE_H_ */