Bug 2067212: Release SourceBuffer waiting consumers outside of mMutex r=azebrowski

Dropping the last reference to a waiting IResumable can destroy an
image::Decoder, whose SourceBufferIterator member reacquires mMutex from
its destructor. Hand the references to the caller so they are released
once mMutex has been dropped.

Differential Revision: https://phabricator.services.mozilla.com/D321935

image/SourceBuffer.cpp

@@ -326,7 +326,8 @@ void SourceBuffer::AddWaitingConsumer(IResumable* aConsumer) {
   }
 }
 
-void SourceBuffer::ResumeWaitingConsumers() {
+void SourceBuffer::ResumeWaitingConsumers(
+    nsTArray<RefPtr<IResumable>>* aOutConsumers) {
   mMutex.AssertCurrentThreadOwns();
 
   if (mWaitingConsumers.Length() == 0) {
@@ -337,12 +338,13 @@ void SourceBuffer::ResumeWaitingConsumers() {
     mWaitingConsumers[i]->Resume();
   }
 
-  mWaitingConsumers.Clear();
+  aOutConsumers->AppendElements(std::move(mWaitingConsumers));
 }
 
 nsresult SourceBuffer::ExpectLength(size_t aExpectedLength) {
   MOZ_ASSERT(aExpectedLength > 0, "Zero expected size?");
 
+  nsTArray<RefPtr<IResumable>> consumers;
   MutexAutoLock lock(mMutex);
 
   if (MOZ_UNLIKELY(mStatus)) {
@@ -357,14 +359,14 @@ nsresult SourceBuffer::ExpectLength(size_t aExpectedLength) {
 
   if (MOZ_UNLIKELY(!SurfaceCache::CanHold(aExpectedLength))) {
     NS_WARNING("SourceBuffer refused to store too large buffer");
-    return HandleError(NS_ERROR_INVALID_ARG);
+    return HandleError(NS_ERROR_INVALID_ARG, &consumers);
   }
 
   size_t length = min(aExpectedLength, MAX_CHUNK_CAPACITY);
   if (MOZ_UNLIKELY(NS_FAILED(AppendChunk(CreateChunk(length,
                                                      /* aExistingCapacity */ 0,
                                                      /* aRoundUp */ false))))) {
-    return HandleError(NS_ERROR_OUT_OF_MEMORY);
+    return HandleError(NS_ERROR_OUT_OF_MEMORY, &consumers);
   }
 
   return NS_OK;
@@ -384,6 +386,7 @@ nsresult SourceBuffer::Append(const char* aData, size_t aLength) {
   size_t forNextChunk = 0;
   size_t nextChunkCapacity = 0;
   size_t totalCapacity = 0;
+  nsTArray<RefPtr<IResumable>> consumers;
 
   {
     MutexAutoLock lock(mMutex);
@@ -395,7 +398,7 @@ nsresult SourceBuffer::Append(const char* aData, size_t aLength) {
 
     if (MOZ_UNLIKELY(mChunks.Length() == 0)) {
       if (MOZ_UNLIKELY(NS_FAILED(AppendChunk(CreateChunk(aLength))))) {
-        return HandleError(NS_ERROR_OUT_OF_MEMORY);
+        return HandleError(NS_ERROR_OUT_OF_MEMORY, &consumers);
       }
     }
 
@@ -454,16 +457,16 @@ nsresult SourceBuffer::Append(const char* aData, size_t aLength) {
     // If we created a new chunk, add it to the series.
     if (forNextChunk > 0) {
       if (MOZ_UNLIKELY(!nextChunk)) {
-        return HandleError(NS_ERROR_OUT_OF_MEMORY);
+        return HandleError(NS_ERROR_OUT_OF_MEMORY, &consumers);
       }
 
       if (MOZ_UNLIKELY(NS_FAILED(AppendChunk(std::move(nextChunk))))) {
-        return HandleError(NS_ERROR_OUT_OF_MEMORY);
+        return HandleError(NS_ERROR_OUT_OF_MEMORY, &consumers);
       }
     }
 
     // Resume any waiting readers now that there's new data.
-    ResumeWaitingConsumers();
+    ResumeWaitingConsumers(&consumers);
   }
 
   return NS_OK;
@@ -532,6 +535,7 @@ nsresult SourceBuffer::AppendFromInputStream(nsIInputStream* aInputStream,
 }
 
 void SourceBuffer::Complete(nsresult aStatus) {
+  nsTArray<RefPtr<IResumable>> consumers;
   MutexAutoLock lock(mMutex);
 
   // When an error occurs internally (e.g. due to an OOM), we save the status.
@@ -552,7 +556,7 @@ void SourceBuffer::Complete(nsresult aStatus) {
   mStatus = Some(aStatus);
 
   // Resume any waiting consumers now that we're complete.
-  ResumeWaitingConsumers();
+  ResumeWaitingConsumers(&consumers);
 
   // If we still have active consumers, just return.
   if (mConsumerCount > 0) {
@@ -707,7 +711,8 @@ SourceBufferIterator::State SourceBuffer::AdvanceIteratorOrScheduleResume(
   return aIterator.SetWaiting(!!aConsumer);
 }
 
-nsresult SourceBuffer::HandleError(nsresult aError) {
+nsresult SourceBuffer::HandleError(
+    nsresult aError, nsTArray<RefPtr<IResumable>>* aOutConsumers) {
   MOZ_ASSERT(NS_FAILED(aError), "Should have an error here");
   MOZ_ASSERT(aError == NS_ERROR_OUT_OF_MEMORY || aError == NS_ERROR_INVALID_ARG,
              "Unexpected error; may want to notify waiting readers, which "
@@ -721,7 +726,7 @@ nsresult SourceBuffer::HandleError(nsresult aError) {
   mStatus = Some(aError);
 
   // Drop our references to waiting readers.
-  mWaitingConsumers.Clear();
+  aOutConsumers->AppendElements(std::move(mWaitingConsumers));
 
   return *mStatus;
 }

image/SourceBuffer.h

@@ -483,7 +483,14 @@ class SourceBuffer final {
   //////////////////////////////////////////////////////////////////////////////
 
   void AddWaitingConsumer(IResumable* aConsumer) MOZ_REQUIRES(mMutex);
-  void ResumeWaitingConsumers() MOZ_REQUIRES(mMutex);
+
+  // Releasing the last reference to a waiting consumer can destroy an
+  // image::Decoder, whose SourceBufferIterator member reacquires mMutex from
+  // its destructor. ResumeWaitingConsumers and HandleError therefore hand our
+  // references to aOutConsumers instead of dropping them; callers must declare
+  // aOutConsumers outside the scope which holds mMutex.
+  void ResumeWaitingConsumers(nsTArray<RefPtr<IResumable>>* aOutConsumers)
+      MOZ_REQUIRES(mMutex);
 
   typedef SourceBufferIterator::State State;
 
@@ -499,7 +506,9 @@ class SourceBuffer final {
   // Helper methods.
   //////////////////////////////////////////////////////////////////////////////
 
-  nsresult HandleError(nsresult aError) MOZ_REQUIRES(mMutex);
+  nsresult HandleError(nsresult aError,
+                       nsTArray<RefPtr<IResumable>>* aOutConsumers)
+      MOZ_REQUIRES(mMutex);
   bool IsEmpty() MOZ_REQUIRES(mMutex);
   bool IsLastChunk(uint32_t aChunk) MOZ_REQUIRES(mMutex);