Bug 1371882 - MediaCache is now ref-counted - r?cpearce draft
authorGerald Squelart <gsquelart@mozilla.com>
Thu, 15 Jun 2017 15:02:25 +1200
changeset 595165 507ffe036c060afb18bba30c3afa0884ee06a99e
parent 595164 17aaf07adeb9a7468c6aeea101b1a53f1a43235e
child 595166 80cd4f7e6c13c04eec2e6ef7e1fb394c7701d2cb
push id64265
push usergsquelart@mozilla.com
push dateFri, 16 Jun 2017 03:37:56 +0000
reviewerscpearce
bugs1371882
milestone56.0a1
Bug 1371882 - MediaCache is now ref-counted - r?cpearce MediaCacheStreams have owning shared pointers to their MediaCache, and a MediaCache owns itself while an update is in flight. A non-owning pointer `gMediaCache` is only used by GetMediaCache and ~MediaCache to manage the one file-backed MediaCache. MozReview-Commit-ID: AQHuXWGrKt6
dom/media/MediaCache.cpp
dom/media/MediaCache.h
--- a/dom/media/MediaCache.cpp
+++ b/dom/media/MediaCache.cpp
@@ -120,38 +120,31 @@ MediaCacheFlusher::UnregisterMediaCache(
 
   gMediaCacheFlusher->mMediaCaches.RemoveElement(aMediaCache);
 
   if (gMediaCacheFlusher->mMediaCaches.Length() == 0) {
     gMediaCacheFlusher = nullptr;
   }
 }
 
-class MediaCache {
+class MediaCache
+{
 public:
+  NS_INLINE_DECL_THREADSAFE_REFCOUNTING(MediaCache)
+
   friend class MediaCacheStream::BlockList;
   typedef MediaCacheStream::BlockList BlockList;
   static const int64_t BLOCK_SIZE = MediaCacheStream::BLOCK_SIZE;
 
   // Get an instance of a MediaCache (or nullptr if initialization failed).
   // aContentLength is the content length if known already, otherwise -1.
   // If the length is known and considered small enough, a discrete MediaCache
   // with memory backing will be given. Otherwise the one MediaCache with
   // file backing will be provided.
-  // The caller adds a reference to the MediaCache object by calling
-  // OpenStream(); and later must release it by calling ReleaseStream() and
-  // then MaybeShutdown().
-  static MediaCache* GetMediaCache(int64_t aContentLength);
-
-  // 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();
+  static RefPtr<MediaCache> GetMediaCache(int64_t aContentLength);
 
   // Brutally flush the cache contents. Main thread only.
   void Flush();
 
   // Close all streams associated with private browsing windows. This will
   // also remove the blocks from the cache since we don't want to leave any
   // traces when PB is done.
   void CloseStreamsForPrivateBrowsing();
@@ -259,27 +252,41 @@ public:
   };
 
 protected:
   explicit MediaCache(int64_t aContentLength)
     : mContentLength(aContentLength)
     , mNextResourceID(1)
     , mReentrantMonitor("MediaCache.mReentrantMonitor")
     , mUpdateQueued(false)
-    , mShutdownInsteadOfUpdating(false)
 #ifdef DEBUG
     , mInUpdate(false)
 #endif
   {
+    NS_ASSERTION(NS_IsMainThread(), "Only construct MediaCache on main thread");
     MOZ_COUNT_CTOR(MediaCache);
     MediaCacheFlusher::RegisterMediaCache(this);
   }
 
   ~MediaCache()
   {
+    NS_ASSERTION(NS_IsMainThread(), "Only destroy MediaCache on main thread");
+    if (this == gMediaCache) {
+      // This is the file-backed MediaCache, reset the global pointer.
+      gMediaCache = nullptr;
+    }
+    if (mContentLength > 0) {
+      // This is an memory-backed MediaCache, update the combined memory usage.
+      gMediaMemoryCachesCombinedSize -= mContentLength;
+      LOG("~MediaCache(Memory-backed MediaCache %p) -> combined size now %" PRIi64,
+          this,
+          gMediaMemoryCachesCombinedSize);
+    } else {
+      LOG("~MediaCache(Global file-backed MediaCache)");
+    }
     MediaCacheFlusher::UnregisterMediaCache(this);
     NS_ASSERTION(mStreams.IsEmpty(), "Stream(s) still open!");
     Truncate();
     NS_ASSERTION(mIndex.Length() == 0, "Blocks leaked?");
     if (mContentLength <= 0) {
       // Only gather "MEDIACACHE" telemetry for the file-based cache.
       LOG("MediaCache::~MediaCache(this=%p) MEDIACACHE_WATERMARK_KB=%u",
           this,
@@ -388,22 +395,20 @@ 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();
-
   // There is at most one file-backed media cache.
+  // It is owned by all MediaCacheStreams that use it.
+  // This is a raw pointer set by GetMediaCache(), and reset by ~MediaCache(),
+  // both on the main thread; and is not accessed anywhere else.
   static MediaCache* gMediaCache;
 
   // Combined size of all memory-backed MediaCaches. Main-thread only.
   static int64_t gMediaMemoryCachesCombinedSize;
 
   // Expected content length if known initially from the HTTP Content-Length
   // header (this is a memory-backed MediaCache), otherwise -1 (file-backed
   // MediaCache).
@@ -427,19 +432,16 @@ 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<MediaBlockCacheBase> mBlockCache;
   // 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;
 };
 
 // Initialized to nullptr by non-local static initialization.
@@ -714,102 +716,54 @@ MediaCache::CloseStreamsForPrivateBrowsi
   MOZ_ASSERT(NS_IsMainThread());
   for (MediaCacheStream* s : mStreams) {
     if (s->mIsPrivateBrowsing) {
       s->Close();
     }
   }
 }
 
-void
-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.
-
-  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).
-    gMediaCache = nullptr;
-  }
-
-  if (mUpdateQueued) {
-    // An update is queued, let it destroy this MediaCache object.
-    mShutdownInsteadOfUpdating = true;
-    return;
-  }
-
-  if (mContentLength > 0) {
-    // This is an memory-backed MediaCache, update the combined memory usage.
-    gMediaMemoryCachesCombinedSize -= mContentLength;
-    LOG("ShutdownAndDestroyThis(Memory-backed MediaCache %p) -> combined size now %" PRIi64,
-        this,
-        gMediaMemoryCachesCombinedSize);
-  } else {
-    LOG("ShutdownAndDestroyThis(Global file-backed MediaCache)");
-  }
-
-  delete this;
-}
-
-/* static */ MediaCache*
+/* static */ RefPtr<MediaCache>
 MediaCache::GetMediaCache(int64_t aContentLength)
 {
   NS_ASSERTION(NS_IsMainThread(), "Only call on main thread");
   static const size_t sysmem = std::max<size_t>(PR_GetPhysicalMemorySize(), 32*1024*1024);
   if (aContentLength > 0 &&
       aContentLength <=
         int64_t(MediaPrefs::MediaMemoryCacheMaxSize()) * 1024 &&
       size_t(gMediaMemoryCachesCombinedSize + aContentLength) <=
         std::min(
         size_t(MediaPrefs::MediaMemoryCachesCombinedLimitKb()) * 1024,
         sysmem * MediaPrefs::MediaMemoryCachesCombinedLimitPcSysmem() / 100)) {
     // Small-enough resource (and we are under the maximum memory usage), use
     // a new memory-backed MediaCache.
-    MediaCache* mc = new MediaCache(aContentLength);
+    RefPtr<MediaCache> mc = new MediaCache(aContentLength);
     nsresult rv = mc->Init();
     if (NS_SUCCEEDED(rv)) {
       gMediaMemoryCachesCombinedSize += aContentLength;
       LOG("GetMediaCache(%" PRIi64
           ") -> Memory MediaCache %p, combined size %" PRIi64,
           aContentLength,
-          mc,
+          mc.get(),
           gMediaMemoryCachesCombinedSize);
       return mc;
     }
     // Memory-backed MediaCache initialization failed, clean up and try for a
     // file-backed MediaCache below.
-    delete mc;
   }
 
   if (gMediaCache) {
     LOG("GetMediaCache(%" PRIi64 ") -> Existing file-backed MediaCache",
         aContentLength);
     return gMediaCache;
   }
 
   gMediaCache = new MediaCache(-1);
   nsresult rv = gMediaCache->Init();
   if (NS_FAILED(rv)) {
-    delete gMediaCache;
     gMediaCache = nullptr;
     LOG("GetMediaCache(%" PRIi64 ") -> Failed to create file-backed MediaCache",
         aContentLength);
   } else {
     LOG("GetMediaCache(%" PRIi64 ") -> Created file-backed MediaCache",
         aContentLength);
   }
 
@@ -1186,24 +1140,16 @@ 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);
@@ -1539,30 +1485,30 @@ MediaCache::Update()
     }
   }
   mSuspendedStatusToNotify.Clear();
 }
 
 class UpdateEvent : public Runnable
 {
 public:
-  explicit UpdateEvent(MediaCache& aMediaCache)
+  explicit UpdateEvent(MediaCache* aMediaCache)
     : Runnable("MediaCache::UpdateEvent")
     , mMediaCache(aMediaCache)
   {
   }
 
   NS_IMETHOD Run() override
   {
-    mMediaCache.Update();
+    mMediaCache->Update();
     return NS_OK;
   }
 
 private:
-  MediaCache& mMediaCache;
+  RefPtr<MediaCache> mMediaCache;
 };
 
 void
 MediaCache::QueueUpdate()
 {
   mReentrantMonitor.AssertCurrentThreadIn();
 
   // Queuing an update while we're in an update raises a high risk of
@@ -1570,17 +1516,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(*this);
+  nsCOMPtr<nsIRunnable> event = new UpdateEvent(this);
   SystemGroup::Dispatch("MediaCache::UpdateEvent",
                         TaskCategory::Other,
                         event.forget());
 }
 
 void
 MediaCache::QueueSuspendedStatusUpdate(int64_t aResourceID)
 {
@@ -2103,17 +2049,16 @@ MediaCacheStream::NotifyChannelRecreated
 MediaCacheStream::~MediaCacheStream()
 {
   NS_ASSERTION(NS_IsMainThread(), "Only call on main thread");
   NS_ASSERTION(!mPinCount, "Unbalanced Pin");
 
   if (mMediaCache) {
     NS_ASSERTION(mClosed, "Stream was not closed");
     mMediaCache->ReleaseStream(this);
-    mMediaCache->MaybeShutdown();
   }
 
   uint32_t lengthKb = uint32_t(
     std::min(std::max(mStreamLength, int64_t(0)) / 1024, int64_t(UINT32_MAX)));
   LOG("MediaCacheStream::~MediaCacheStream(this=%p) "
       "MEDIACACHESTREAM_LENGTH_KB=%" PRIu32,
       this,
       lengthKb);
--- a/dom/media/MediaCache.h
+++ b/dom/media/MediaCache.h
@@ -435,17 +435,17 @@ private:
   // aReentrantMonitor is the nsAutoReentrantMonitor wrapper holding the cache monitor.
   // This is used to NotifyAll to wake up threads that might be
   // blocked on reading from this stream.
   void CloseInternal(ReentrantMonitorAutoEnter& aReentrantMonitor);
   // Update mPrincipal given that data has been received from aPrincipal
   bool UpdatePrincipal(nsIPrincipal* aPrincipal);
 
   // Instance of MediaCache to use with this MediaCacheStream.
-  MediaCache* mMediaCache;
+  RefPtr<MediaCache> mMediaCache;
 
   // These fields are main-thread-only.
   ChannelMediaResource*  mClient;
   nsCOMPtr<nsIPrincipal> mPrincipal;
   // Set to true when MediaCache::Update() has finished while this stream
   // was present.
   bool                   mHasHadUpdate;
   // Set to true when the stream has been closed either explicitly or