Bug 1113282 - Check during async TrackBuffer decoder initialization to make sure we haven't been shut down. r=cajbir
authorBobby Holley <bobbyholley@gmail.com>
Thu, 18 Dec 2014 13:59:00 +0100
changeset 220582 e7dbd3eb21e5201a4e2985b4a2683fd60d1cb4cd
parent 220581 d8af69e0a429ac27627b83fba21ded2e25c45ba0
child 220583 3233b239e61d238c50f28e2949791de63ffffb9f
push id10503
push userryanvm@gmail.com
push dateFri, 19 Dec 2014 20:13:42 +0000
treeherderfx-team@98ee95ac6be5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscajbir
bugs1113282
milestone37.0a1
Bug 1113282 - Check during async TrackBuffer decoder initialization to make sure we haven't been shut down. r=cajbir
dom/media/mediasource/MediaSourceReader.cpp
dom/media/mediasource/TrackBuffer.cpp
dom/media/mediasource/TrackBuffer.h
--- a/dom/media/mediasource/MediaSourceReader.cpp
+++ b/dom/media/mediasource/MediaSourceReader.cpp
@@ -322,16 +322,17 @@ MediaSourceReader::Shutdown()
 
   ContinueShutdown();
   return p;
 }
 
 void
 MediaSourceReader::ContinueShutdown()
 {
+  ReentrantMonitorAutoEnter mon(mDecoder->GetReentrantMonitor());
   if (mTrackBuffers.Length()) {
     mTrackBuffers[0]->Shutdown()->Then(GetTaskQueue(), __func__, this,
                                        &MediaSourceReader::ContinueShutdown,
                                        &MediaSourceReader::ContinueShutdown);
     mShutdownTrackBuffers.AppendElement(mTrackBuffers[0]);
     mTrackBuffers.RemoveElementAt(0);
     return;
   }
--- a/dom/media/mediasource/TrackBuffer.cpp
+++ b/dom/media/mediasource/TrackBuffer.cpp
@@ -34,22 +34,24 @@ extern PRLogModuleInfo* GetMediaSourceAP
 #endif
 
 namespace mozilla {
 
 TrackBuffer::TrackBuffer(MediaSourceDecoder* aParentDecoder, const nsACString& aType)
   : mParentDecoder(aParentDecoder)
   , mType(aType)
   , mLastStartTimestamp(0)
+  , mShutdown(false)
 {
   MOZ_COUNT_CTOR(TrackBuffer);
   mParser = ContainerParser::CreateForMIMEType(aType);
   mTaskQueue = new MediaTaskQueue(GetMediaDecodeThreadPool());
   aParentDecoder->AddTrackBuffer(this);
   mDecoderPerSegment = Preferences::GetBool("media.mediasource.decoder-per-segment", false);
+  MSE_DEBUG("TrackBuffer(%p) created for parent decoder %p", this, aParentDecoder);
 }
 
 TrackBuffer::~TrackBuffer()
 {
   MOZ_COUNT_DTOR(TrackBuffer);
 }
 
 class ReleaseDecoderTask : public nsRunnable {
@@ -96,16 +98,19 @@ public:
 private:
   TrackBuffer* mOwner;
   nsAutoTArray<nsRefPtr<SourceBufferDecoder>,2> mDecoders;
 };
 
 nsRefPtr<ShutdownPromise>
 TrackBuffer::Shutdown()
 {
+  mParentDecoder->GetReentrantMonitor().AssertCurrentThreadIn();
+  mShutdown = true;
+
   MOZ_ASSERT(mShutdownPromise.IsEmpty());
   nsRefPtr<ShutdownPromise> p = mShutdownPromise.Ensure(__func__);
 
   RefPtr<MediaTaskQueue> queue = mTaskQueue;
   mTaskQueue = nullptr;
   queue->BeginShutdown()
        ->Then(mParentDecoder->GetReader()->GetTaskQueue(), __func__, this,
               &TrackBuffer::ContinueShutdown, &TrackBuffer::ContinueShutdown);
@@ -365,33 +370,52 @@ TrackBuffer::QueueInitializeDecoder(Sour
     return false;
   }
   return true;
 }
 
 void
 TrackBuffer::InitializeDecoder(SourceBufferDecoder* aDecoder)
 {
-  MOZ_ASSERT(mTaskQueue->IsCurrentThreadIn());
+  // ReadMetadata may block the thread waiting on data, so we must be able
+  // to leave the monitor while we call it. For the rest of this function
+  // we want to hold the monitor though, since we run on a different task queue
+  // from the reader and interact heavily with it.
+  mParentDecoder->GetReentrantMonitor().AssertNotCurrentThreadIn();
+  ReentrantMonitorAutoEnter mon(mParentDecoder->GetReentrantMonitor());
 
-  // ReadMetadata may block the thread waiting on data, so it must not be
-  // called with the monitor held.
-  mParentDecoder->GetReentrantMonitor().AssertNotCurrentThreadIn();
+  // We may be shut down at any time by the reader on another thread. So we need
+  // to check for this each time we acquire the monitor. If that happens, we
+  // need to abort immediately, because the reader has forgotten about us, and
+  // important pieces of our state (like mTaskQueue) have also been torn down.
+  if (mShutdown) {
+    MSE_DEBUG("TrackBuffer(%p) was shut down. Aborting initialization.", this);
+    return;
+  }
 
+  MOZ_ASSERT(mTaskQueue->IsCurrentThreadIn());
   MediaDecoderReader* reader = aDecoder->GetReader();
   MSE_DEBUG("TrackBuffer(%p): Initializing subdecoder %p reader %p",
             this, aDecoder, reader);
 
   MediaInfo mi;
   nsAutoPtr<MetadataTags> tags; // TODO: Handle metadata.
-  nsresult rv = reader->ReadMetadata(&mi, getter_Transfers(tags));
+  nsresult rv;
+  {
+    ReentrantMonitorAutoExit mon(mParentDecoder->GetReentrantMonitor());
+    rv = reader->ReadMetadata(&mi, getter_Transfers(tags));
+  }
+
   reader->SetIdle();
+  if (mShutdown) {
+    MSE_DEBUG("TrackBuffer(%p) was shut down while reading metadata. Aborting initialization.", this);
+    return;
+  }
 
   if (NS_SUCCEEDED(rv) && reader->IsWaitingOnCDMResource()) {
-    ReentrantMonitorAutoEnter mon(mParentDecoder->GetReentrantMonitor());
     mWaitingDecoders.AppendElement(aDecoder);
     return;
   }
 
   aDecoder->SetTaskQueue(nullptr);
 
   if (NS_FAILED(rv) || (!mi.HasVideo() && !mi.HasAudio())) {
     // XXX: Need to signal error back to owning SourceBuffer.
@@ -437,17 +461,17 @@ TrackBuffer::ValidateTrackFormats(const 
   }
 
   return true;
 }
 
 bool
 TrackBuffer::RegisterDecoder(SourceBufferDecoder* aDecoder)
 {
-  ReentrantMonitorAutoEnter mon(mParentDecoder->GetReentrantMonitor());
+  mParentDecoder->GetReentrantMonitor().AssertCurrentThreadIn();
   const MediaInfo& info = aDecoder->GetReader()->GetMediaInfo();
   // Initialize the track info since this is the first decoder.
   if (mInitializedDecoders.IsEmpty()) {
     mInfo = info;
     mParentDecoder->OnTrackBufferConfigured(this, mInfo);
   }
   if (!ValidateTrackFormats(info)) {
     MSE_DEBUG("TrackBuffer(%p)::RegisterDecoder with mismatched audio/video tracks", this);
--- a/dom/media/mediasource/TrackBuffer.h
+++ b/dom/media/mediasource/TrackBuffer.h
@@ -158,12 +158,13 @@ private:
 
   // Set when the first decoder used by this TrackBuffer is initialized.
   // Protected by mParentDecoder's monitor.
   MediaInfo mInfo;
 
   void ContinueShutdown();
   MediaPromiseHolder<ShutdownPromise> mShutdownPromise;
   bool mDecoderPerSegment;
+  bool mShutdown;
 };
 
 } // namespace mozilla
 #endif /* MOZILLA_TRACKBUFFER_H_ */