Bug 1354389. P1 - drop the monitor before doing IO. r?cpearce draft
authorJW Wang <jwwang@mozilla.com>
Mon, 10 Apr 2017 11:55:26 +0800
changeset 559513 04bacf90f86d7d34a0b0ffdd4aa80c53c2f04804
parent 559512 2d6609b6d231d1c033cd7c85e5cf1b11021aa22a
child 559514 ae703bffa62d467e7bb9b2bda2754e8c463d19c1
push id53117
push userjwwang@mozilla.com
push dateMon, 10 Apr 2017 08:16:18 +0000
reviewerscpearce
bugs1354389
milestone55.0a1
Bug 1354389. P1 - drop the monitor before doing IO. r?cpearce We don't want to do IO while holding the monitor which might be acquired on the main thread in MediaCache::Update(). MozReview-Commit-ID: 4cHM35K8Twb
dom/media/MediaCache.cpp
--- a/dom/media/MediaCache.cpp
+++ b/dom/media/MediaCache.cpp
@@ -143,16 +143,18 @@ public:
   // 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.
   static void CloseStreamsForPrivateBrowsing();
 
   // Cache-file access methods. These are the lowest-level cache methods.
   // mReentrantMonitor must be held; these can be called on any thread.
   // This can return partial reads.
+  // Note mReentrantMonitor will be dropped while doing IO. The caller need
+  // to handle changes happening when the monitor is not held.
   nsresult ReadCacheFile(int64_t aOffset, void* aData, int32_t aLength,
                          int32_t* aBytes);
   // This will fail if all aLength bytes are not read
   nsresult ReadCacheFileAllBytes(int64_t aOffset, void* aData, int32_t aLength);
 
   int64_t AllocateResourceID()
   {
     mReentrantMonitor.AssertCurrentThreadIn();
@@ -663,25 +665,33 @@ InitMediaCache()
   nsresult rv = gMediaCache->Init();
   if (NS_FAILED(rv)) {
     delete gMediaCache;
     gMediaCache = nullptr;
   }
 }
 
 nsresult
-MediaCache::ReadCacheFile(int64_t aOffset, void* aData, int32_t aLength,
-                            int32_t* aBytes)
+MediaCache::ReadCacheFile(
+  int64_t aOffset, void* aData, int32_t aLength, int32_t* aBytes)
 {
   mReentrantMonitor.AssertCurrentThreadIn();
-
-  if (!mFileCache)
+  RefPtr<FileBlockCache> fileCache = mFileCache;
+  if (!fileCache) {
     return NS_ERROR_FAILURE;
-
-  return mFileCache->Read(aOffset, reinterpret_cast<uint8_t*>(aData), aLength, aBytes);
+  }
+  {
+    // Since the monitor might be acquired on the main thread, we need to drop
+    // the monitor while doing IO in order not to block the main thread.
+    // TODO: fix calls of ReadCacheFile() to handle state changes happening when
+    // the monitor is not held.
+    ReentrantMonitorAutoExit unlock(mReentrantMonitor);
+    return fileCache->Read(aOffset,
+      reinterpret_cast<uint8_t*>(aData), aLength, aBytes);
+  }
 }
 
 nsresult
 MediaCache::ReadCacheFileAllBytes(int64_t aOffset, void* aData, int32_t aLength)
 {
   mReentrantMonitor.AssertCurrentThreadIn();
 
   int64_t offset = aOffset;