Bug 1371882 - Delay MediaCache destruction if update queued - r?cpearce draft
authorGerald Squelart <gsquelart@mozilla.com>
Thu, 08 Jun 2017 16:45:21 +1200
changeset 593026 2b5f8fd564b585d8910e31e9ae01ee80b8eec820
parent 593025 f6b8a2b1e245edf5c60641ca919e6b0cebde66dc
child 593027 a52e16c02b50e12772ae3f9b2685e565471f6b1b
push id63579
push usergsquelart@mozilla.com
push dateTue, 13 Jun 2017 05:16:04 +0000
reviewerscpearce
bugs1371882
milestone56.0a1
Bug 1371882 - Delay MediaCache destruction if update queued - r?cpearce MozReview-Commit-ID: LIjZFUIrTtX
dom/media/MediaCache.cpp
--- a/dom/media/MediaCache.cpp
+++ b/dom/media/MediaCache.cpp
@@ -130,17 +130,17 @@ public:
   friend class MediaCacheStream::BlockList;
   typedef MediaCacheStream::BlockList BlockList;
   static const int64_t BLOCK_SIZE = MediaCacheStream::BLOCK_SIZE;
 
   // Get an instance of the shared MediaCache.
   // Returns nullptr if initialization failed.
   static MediaCache* GetMediaCache();
 
-  // Shut down the global cache if it's no longer needed. We shut down
+  // Shut down the cache if it's no longer needed. We shut down
   // the cache as soon as there are no streams. This means that during
   // normal operation we are likely to start up the cache and shut it down
   // many times, but that's OK since starting it up is cheap and
   // shutting it down cleans things up and releases disk space.
   void MaybeShutdown();
 
   // Brutally flush the cache contents. Main thread only.
   void Flush();
@@ -247,16 +247,17 @@ public:
     uint32_t mNext;
   };
 
 protected:
   MediaCache()
     : mNextResourceID(1)
     , mReentrantMonitor("MediaCache.mReentrantMonitor")
     , mUpdateQueued(false)
+    , mShutdownInsteadOfUpdating(false)
 #ifdef DEBUG
     , mInUpdate(false)
 #endif
   {
     MOZ_COUNT_CTOR(MediaCache);
     MediaCacheFlusher::RegisterMediaCache(this);
   }
 
@@ -372,16 +373,21 @@ protected:
   TimeDuration PredictNextUse(TimeStamp aNow, int32_t aBlock);
   // Guess the duration until the next incoming data on aStream will be used
   TimeDuration PredictNextUseForIncomingData(MediaCacheStream* aStream);
 
   // Truncate the file and index array if there are free blocks at the
   // end
   void Truncate();
 
+  // Shutdown this MediaCache, and reset gMediaCache if we are the global one.
+  // If there is no queued update, destroy the MediaCache immediately.
+  // Otherwise when the update is processed, it will destroy the MediaCache.
+  void ShutdownAndDestroyThis();
+
   // This member is main-thread only. It's used to allocate unique
   // resource IDs to streams.
   int64_t                       mNextResourceID;
 
   // The monitor protects all the data members here. Also, off-main-thread
   // readers that need to block will Wait() on this monitor. When new
   // data becomes available in the cache, we NotifyAll() on this monitor.
   ReentrantMonitor         mReentrantMonitor;
@@ -395,16 +401,19 @@ protected:
   // Keep track for highest number of blocks owners, for telemetry purposes.
   uint32_t mBlockOwnersWatermark = 0;
   // Writer which performs IO, asynchronously writing cache blocks.
   RefPtr<FileBlockCache> mFileCache;
   // The list of free blocks; they are not ordered.
   BlockList       mFreeBlocks;
   // True if an event to run Update() has been queued but not processed
   bool            mUpdateQueued;
+  // Main-thread only. True when shutting down, and the update task should
+  // destroy this MediaCache.
+  bool            mShutdownInsteadOfUpdating;
 #ifdef DEBUG
   bool            mInUpdate;
 #endif
   // A list of resource IDs to notify about the change in suspended status.
   nsTArray<int64_t> mSuspendedStatusToNotify;
 };
 
 NS_IMETHODIMP
@@ -681,21 +690,41 @@ MediaCache::MaybeShutdown()
 {
   NS_ASSERTION(NS_IsMainThread(),
                "MediaCache::MaybeShutdown called on non-main thread");
   if (!mStreams.IsEmpty()) {
     // Don't shut down yet, streams are still alive
     return;
   }
 
+  ShutdownAndDestroyThis();
+}
+
+void
+MediaCache::ShutdownAndDestroyThis()
+{
+  NS_ASSERTION(NS_IsMainThread(),
+               "MediaCache::Shutdown called on non-main thread");
   // Since we're on the main thread, no-one is going to add a new stream
   // while we shut down.
-  // This function is static so we don't have to delete 'this'.
-  delete gMediaCache;
-  gMediaCache = nullptr;
+
+  if (this == gMediaCache) {
+    // This is the global MediaCache, reset the global pointer to ensure it's
+    // not used anymore (in case it doesn't get destroyed immediately).
+    MOZ_ASSERT(mContentLength <= 0);
+    gMediaCache = nullptr;
+  }
+
+  if (mUpdateQueued) {
+    // An update is queued, let it destroy this MediaCache object.
+    mShutdownInsteadOfUpdating = true;
+    return;
+  }
+
+  delete this;
 }
 
 /* static */ MediaCache*
 MediaCache::GetMediaCache()
 {
   NS_ASSERTION(NS_IsMainThread(), "Only call on main thread");
   if (gMediaCache) {
     return gMediaCache;
@@ -1081,16 +1110,24 @@ MediaCache::PredictNextUseForIncomingDat
 
 enum StreamAction { NONE, SEEK, SEEK_AND_RESUME, RESUME, SUSPEND };
 
 void
 MediaCache::Update()
 {
   NS_ASSERTION(NS_IsMainThread(), "Only call on main thread");
 
+  if (mShutdownInsteadOfUpdating) {
+    // Safe to modify mUpdateQueued as we are shutting down and nobody should
+    // call MediaCache functions now.
+    mUpdateQueued = false;
+    ShutdownAndDestroyThis();
+    return;
+  }
+
   // The action to use for each stream. We store these so we can make
   // decisions while holding the cache lock but implement those decisions
   // without holding the cache lock, since we need to call out to
   // stream, decoder and element code.
   AutoTArray<StreamAction,10> actions;
 
   {
     ReentrantMonitorAutoEnter mon(mReentrantMonitor);
@@ -1426,25 +1463,30 @@ MediaCache::Update()
     }
   }
   mSuspendedStatusToNotify.Clear();
 }
 
 class UpdateEvent : public Runnable
 {
 public:
-  UpdateEvent() : Runnable("MediaCache::UpdateEvent") {}
+  explicit UpdateEvent(MediaCache& aMediaCache)
+    : Runnable("MediaCache::UpdateEvent")
+    , mMediaCache(aMediaCache)
+  {
+  }
 
   NS_IMETHOD Run() override
   {
-    if (gMediaCache) {
-      gMediaCache->Update();
-    }
+    mMediaCache.Update();
     return NS_OK;
   }
+
+private:
+  MediaCache& mMediaCache;
 };
 
 void
 MediaCache::QueueUpdate()
 {
   mReentrantMonitor.AssertCurrentThreadIn();
 
   // Queuing an update while we're in an update raises a high risk of
@@ -1452,17 +1494,17 @@ MediaCache::QueueUpdate()
   NS_ASSERTION(!mInUpdate,
                "Queuing an update while we're in an update");
   if (mUpdateQueued)
     return;
   mUpdateQueued = true;
   // XXX MediaCache does updates when decoders are still running at
   // shutdown and get freed in the final cycle-collector cleanup.  So
   // don't leak a runnable in that case.
-  nsCOMPtr<nsIRunnable> event = new UpdateEvent();
+  nsCOMPtr<nsIRunnable> event = new UpdateEvent(*this);
   SystemGroup::Dispatch("MediaCache::UpdateEvent",
                         TaskCategory::Other,
                         event.forget());
 }
 
 void
 MediaCache::QueueSuspendedStatusUpdate(int64_t aResourceID)
 {