Bug 1428242. P3 - use a non-reentrant monitor. draft
authorJW Wang <jwwang@mozilla.com>
Fri, 05 Jan 2018 10:49:38 +0800
changeset 716099 aeab8a218ab098a7d7cbe60c44829e17ac5428e6
parent 716098 e5489e86f5debf8d2ff0b96b81a8d0c20eab7c0e
child 716142 9838c1f487a9c3e97b4e67921803911dc42dd810
push id94337
push userjwwang@mozilla.com
push dateFri, 05 Jan 2018 06:34:48 +0000
bugs1428242
milestone59.0a1
Bug 1428242. P3 - use a non-reentrant monitor. MozReview-Commit-ID: GCXBHugTLJV
dom/media/MediaCache.cpp
dom/media/MediaCache.h
--- a/dom/media/MediaCache.cpp
+++ b/dom/media/MediaCache.cpp
@@ -11,18 +11,18 @@
 #include "MediaBlockCacheBase.h"
 #include "MediaPrefs.h"
 #include "MediaResource.h"
 #include "MemoryBlockCache.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/ClearOnShutdown.h"
 #include "mozilla/ErrorNames.h"
 #include "mozilla/Logging.h"
+#include "mozilla/Monitor.h"
 #include "mozilla/Preferences.h"
-#include "mozilla/ReentrantMonitor.h"
 #include "mozilla/Services.h"
 #include "mozilla/StaticPtr.h"
 #include "mozilla/SystemGroup.h"
 #include "mozilla/Telemetry.h"
 #include "nsContentUtils.h"
 #include "nsIObserverService.h"
 #include "nsIPrincipal.h"
 #include "nsPrintfCString.h"
@@ -137,18 +137,17 @@ MediaCacheFlusher::UnregisterMediaCache(
 
   if (gMediaCacheFlusher->mMediaCaches.Length() == 0) {
     gMediaCacheFlusher = nullptr;
   }
 }
 
 class MediaCache
 {
-  using AutoLock = ReentrantMonitorAutoEnter;
-  using AutoUnlock = ReentrantMonitorAutoExit;
+  using AutoLock = MonitorAutoLock;
 
 public:
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(MediaCache)
 
   friend class MediaCacheStream::BlockList;
   typedef MediaCacheStream::BlockList BlockList;
   static const int64_t BLOCK_SIZE = MediaCacheStream::BLOCK_SIZE;
 
@@ -243,17 +242,17 @@ public:
 
 #ifdef DEBUG_VERIFY_CACHE
   // Verify invariants, especially block list invariants
   void Verify(AutoLock&);
 #else
   void Verify(AutoLock&) {}
 #endif
 
-  ReentrantMonitor& Monitor()
+  mozilla::Monitor& Monitor()
   {
     MOZ_DIAGNOSTIC_ASSERT(!NS_IsMainThread());
     return mMonitor;
   }
 
   /**
    * An iterator that makes it easy to iterate through all streams that
    * have a given resource ID and are not closed.
@@ -262,17 +261,17 @@ public:
   class ResourceStreamIterator
   {
   public:
     ResourceStreamIterator(MediaCache* aMediaCache, int64_t aResourceID)
       : mMediaCache(aMediaCache)
       , mResourceID(aResourceID)
       , mNext(0)
     {
-      aMediaCache->mMonitor.AssertCurrentThreadIn();
+      aMediaCache->mMonitor.AssertCurrentThreadOwns();
     }
     MediaCacheStream* Next(AutoLock& aLock)
     {
       while (mNext < mMediaCache->mStreams.Length()) {
         MediaCacheStream* stream = mMediaCache->mStreams[mNext];
         ++mNext;
         if (stream->GetResourceID() == mResourceID && !stream->IsClosed(aLock))
           return stream;
@@ -442,17 +441,17 @@ protected:
 
   // This member is main-thread only. It's used to allocate unique
   // resource IDs to streams.
   int64_t mNextResourceID = 0;
 
   // 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 mMonitor;
+  mozilla::Monitor mMonitor;
   // This must always be accessed when the monitor is held.
   nsTArray<MediaCacheStream*> mStreams;
   // The Blocks describing the cache entries.
   nsTArray<Block> mIndex;
   // Keep track for highest number of blocks used, for telemetry purposes.
   int32_t mIndexWatermark = 0;
   // Keep track for highest number of blocks owners, for telemetry purposes.
   uint32_t mBlockOwnersWatermark = 0;
@@ -2666,20 +2665,19 @@ MediaCacheStream::ReadBlockFromCache(Aut
     mMediaCache->NoteBlockUsage(
       aLock, this, cacheBlock, aOffset, mCurrentMode, TimeStamp::Now());
   }
 
   return bytesRead;
 }
 
 nsresult
-MediaCacheStream::Read(char* aBuffer, uint32_t aCount, uint32_t* aBytes)
+MediaCacheStream::Read(AutoLock& aLock, char* aBuffer, uint32_t aCount, uint32_t* aBytes)
 {
   MOZ_ASSERT(!NS_IsMainThread());
-  AutoLock lock(mMediaCache->Monitor());
 
   // Cache the offset in case it is changed again when we are waiting for the
   // monitor to be notified to avoid reading at the wrong position.
   auto streamOffset = mStreamOffset;
 
   // The buffer we are about to fill.
   auto buffer = MakeSpan<char>(aBuffer, aCount);
 
@@ -2695,17 +2693,17 @@ MediaCacheStream::Read(char* aBuffer, ui
     }
 
     if (mStreamLength >= 0 && streamOffset >= mStreamLength) {
       // Don't try to read beyond the end of the stream
       break;
     }
 
     Result<uint32_t, nsresult> rv = ReadBlockFromCache(
-      lock, streamOffset, buffer, true /* aNoteBlockUsage */);
+      aLock, streamOffset, buffer, true /* aNoteBlockUsage */);
     if (rv.isErr()) {
       return rv.unwrapErr();
     }
 
     uint32_t bytes = rv.unwrap();
     if (bytes > 0) {
       // Got data from the cache successfully. Read next block.
       streamOffset += bytes;
@@ -2713,78 +2711,78 @@ MediaCacheStream::Read(char* aBuffer, ui
       continue;
     }
 
     // See if we can use the data in the partial block of any stream reading
     // this resource. Note we use the partial block only when it is completed,
     // that is reaching EOS.
     bool foundDataInPartialBlock = false;
     MediaCache::ResourceStreamIterator iter(mMediaCache, mResourceID);
-    while (MediaCacheStream* stream = iter.Next(lock)) {
+    while (MediaCacheStream* stream = iter.Next(aLock)) {
       if (OffsetToBlockIndexUnchecked(stream->mChannelOffset) ==
             OffsetToBlockIndexUnchecked(streamOffset) &&
           stream->mChannelOffset == stream->mStreamLength) {
-        uint32_t bytes = stream->ReadPartialBlock(lock, streamOffset, buffer);
+        uint32_t bytes = stream->ReadPartialBlock(aLock, streamOffset, buffer);
         streamOffset += bytes;
         buffer = buffer.From(bytes);
         foundDataInPartialBlock = true;
         break;
       }
     }
     if (foundDataInPartialBlock) {
       // Break for we've reached EOS.
       break;
     }
 
     if (mDidNotifyDataEnded && NS_FAILED(mNotifyDataEndedStatus)) {
       // Since download ends abnormally, there is no point in waiting for new
       // data to come. We will check the partial block to read as many bytes as
       // possible before exiting this function.
-      bytes = ReadPartialBlock(lock, streamOffset, buffer);
+      bytes = ReadPartialBlock(aLock, streamOffset, buffer);
       streamOffset += bytes;
       buffer = buffer.From(bytes);
       break;
     }
 
     if (mStreamOffset != streamOffset) {
       // Update mStreamOffset before we drop the lock. We need to run
       // Update() again since stream reading strategy might have changed.
       mStreamOffset = streamOffset;
-      mMediaCache->QueueUpdate(lock);
+      mMediaCache->QueueUpdate(aLock);
     }
 
     // No data to read, so block
-    lock.Wait();
+    aLock.Wait();
     continue;
   }
 
   uint32_t count = buffer.Elements() - aBuffer;
   *aBytes = count;
   if (count == 0) {
     return NS_OK;
   }
 
   // Some data was read, so queue an update since block priorities may
   // have changed
-  mMediaCache->QueueUpdate(lock);
+  mMediaCache->QueueUpdate(aLock);
 
   LOG("Stream %p Read at %" PRId64 " count=%d", this, streamOffset-count, count);
   mStreamOffset = streamOffset;
   return NS_OK;
 }
 
 nsresult
 MediaCacheStream::ReadAt(int64_t aOffset, char* aBuffer,
                          uint32_t aCount, uint32_t* aBytes)
 {
   MOZ_ASSERT(!NS_IsMainThread());
   AutoLock lock(mMediaCache->Monitor());
   nsresult rv = Seek(lock, aOffset);
   if (NS_FAILED(rv)) return rv;
-  return Read(aBuffer, aCount, aBytes);
+  return Read(lock, aBuffer, aCount, aBytes);
 }
 
 nsresult
 MediaCacheStream::ReadFromCache(char* aBuffer, int64_t aOffset, uint32_t aCount)
 {
   MOZ_ASSERT(!NS_IsMainThread());
   AutoLock lock(mMediaCache->Monitor());
 
--- a/dom/media/MediaCache.h
+++ b/dom/media/MediaCache.h
@@ -21,17 +21,17 @@
 class nsIEventTarget;
 class nsIPrincipal;
 
 namespace mozilla {
 // defined in MediaResource.h
 class ChannelMediaResource;
 typedef media::IntervalSet<int64_t> MediaByteRangeSet;
 class MediaResource;
-class ReentrantMonitorAutoEnter;
+class MonitorAutoLock;
 
 /**
  * Media applications want fast, "on demand" random access to media data,
  * for pausing, seeking, etc. But we are primarily interested
  * in transporting media data using HTTP over the Internet, which has
  * high latency to open a connection, requires a new connection for every
  * seek, may not even support seeking on some connections (especially
  * live streams), and uses a push model --- data comes from the server
@@ -187,17 +187,17 @@ DDLoggedTypeDeclName(MediaCacheStream);
 /**
  * If the cache fails to initialize then Init will fail, so nonstatic
  * methods of this class can assume gMediaCache is non-null.
  *
  * This class can be directly embedded as a value.
  */
 class MediaCacheStream : public DecoderDoctorLifeLogger<MediaCacheStream>
 {
-  using AutoLock = ReentrantMonitorAutoEnter;
+  using AutoLock = MonitorAutoLock;
 
 public:
   // This needs to be a power of two
   static const int64_t BLOCK_SIZE = 32768;
 
   enum ReadMode {
     MODE_METADATA,
     MODE_PLAYBACK
@@ -345,17 +345,17 @@ public:
 
   // These methods must be called on a different thread from the main
   // thread. They should always be called on the same thread for a given
   // stream.
   // *aBytes gets the number of bytes that were actually read. This can
   // be less than aCount. If the first byte of data is not in the cache,
   // this will block until the data is available or the stream is
   // closed, otherwise it won't block.
-  nsresult Read(char* aBuffer, uint32_t aCount, uint32_t* aBytes);
+  nsresult Read(AutoLock&, char* aBuffer, uint32_t aCount, uint32_t* aBytes);
   // Seeks to aOffset in the stream then performs a Read operation. See
   // 'Read' for argument and return details.
   nsresult ReadAt(int64_t aOffset, char* aBuffer,
                   uint32_t aCount, uint32_t* aBytes);
 
   void ThrottleReadahead(bool bThrottle);
 
   size_t SizeOfExcludingThis(MallocSizeOf aMallocSizeOf) const;