Bug 1196112. Part 1 - ensure all members except |mShuttingDown| and |mOutputStreamManager| are accessed on the worker thread only. r=roc.
☠☠ backed out by 49879f964c82 ☠ ☠
authorJW Wang <jwwang@mozilla.com>
Mon, 24 Aug 2015 21:05:22 +0800
changeset 259031 c678e1317fa0b592169260b7e98a8985abc267ec
parent 259030 3e0ba8a66ed401eb3b423fa7d4c4a69763df5e09
child 259032 b2eb913e58c9c7ddeddb7bdfb95e0846498da514
push id29268
push userryanvm@gmail.com
push dateTue, 25 Aug 2015 00:37:23 +0000
treeherdermozilla-central@08015770c9d6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersroc
bugs1196112
milestone43.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1196112. Part 1 - ensure all members except |mShuttingDown| and |mOutputStreamManager| are accessed on the worker thread only. r=roc.
dom/media/DecodedStream.cpp
dom/media/DecodedStream.h
dom/media/MediaDecoderStateMachine.h
--- a/dom/media/DecodedStream.cpp
+++ b/dom/media/DecodedStream.cpp
@@ -100,17 +100,17 @@ UpdateStreamBlocking(MediaStream* aStrea
  * We have at most one DecodedStreamDaata per MediaDecoder. Its stream
  * is used as the input for each ProcessedMediaStream created by calls to
  * captureStream(UntilEnded). Seeking creates a new source stream, as does
  * replaying after the input as ended. In the latter case, the new source is
  * not connected to streams created by captureStreamUntilEnded.
  */
 class DecodedStreamData {
 public:
-  DecodedStreamData(SourceMediaStream* aStream, bool aPlaying,
+  DecodedStreamData(SourceMediaStream* aStream,
                     MozPromiseHolder<GenericPromise>&& aPromise);
   ~DecodedStreamData();
   bool IsFinished() const;
   int64_t GetPosition() const;
   void SetPlaying(bool aPlaying);
 
   /* The following group of fields are protected by the decoder's monitor
    * and can be read or written on any thread.
@@ -137,37 +137,35 @@ public:
   const nsRefPtr<SourceMediaStream> mStream;
   nsRefPtr<DecodedStreamGraphListener> mListener;
   bool mPlaying;
   // True if we need to send a compensation video frame to ensure the
   // StreamTime going forward.
   bool mEOSVideoCompensation;
 };
 
-DecodedStreamData::DecodedStreamData(SourceMediaStream* aStream, bool aPlaying,
+DecodedStreamData::DecodedStreamData(SourceMediaStream* aStream,
                                      MozPromiseHolder<GenericPromise>&& aPromise)
   : mAudioFramesWritten(0)
   , mNextVideoTime(-1)
   , mNextAudioTime(-1)
   , mStreamInitialized(false)
   , mHaveSentFinish(false)
   , mHaveSentFinishAudio(false)
   , mHaveSentFinishVideo(false)
   , mStream(aStream)
-  , mPlaying(aPlaying)
+  , mPlaying(true)
   , mEOSVideoCompensation(false)
 {
   // DecodedStreamGraphListener will resolve this promise.
   mListener = new DecodedStreamGraphListener(mStream, Move(aPromise));
   mStream->AddListener(mListener);
 
-  // Block the stream if we are not playing.
-  if (!aPlaying) {
-    UpdateStreamBlocking(mStream, true);
-  }
+  // mPlaying is initially true because MDSM won't start playback until playing
+  // becomes true. This is consistent with the settings of AudioSink.
 }
 
 DecodedStreamData::~DecodedStreamData()
 {
   mListener->Forget();
   mStream->Destroy();
 }
 
@@ -353,29 +351,37 @@ OutputStreamManager::Disconnect()
     }
   }
 }
 
 DecodedStream::DecodedStream(AbstractThread* aOwnerThread,
                              MediaQueue<MediaData>& aAudioQueue,
                              MediaQueue<MediaData>& aVideoQueue)
   : mOwnerThread(aOwnerThread)
+  , mShuttingDown(false)
   , mMonitor("DecodedStream::mMonitor")
   , mPlaying(false)
   , mVolume(1.0)
   , mAudioQueue(aAudioQueue)
   , mVideoQueue(aVideoQueue)
 {
 }
 
 DecodedStream::~DecodedStream()
 {
   MOZ_ASSERT(mStartTime.isNothing(), "playback should've ended.");
 }
 
+void
+DecodedStream::Shutdown()
+{
+  MOZ_ASSERT(NS_IsMainThread());
+  mShuttingDown = true;
+}
+
 nsRefPtr<GenericPromise>
 DecodedStream::StartPlayback(int64_t aStartTime, const MediaInfo& aInfo)
 {
   AssertOwnerThread();
   ReentrantMonitorAutoEnter mon(GetReentrantMonitor());
   MOZ_ASSERT(mStartTime.isNothing(), "playback already started.");
 
   mStartTime.emplace(aStartTime);
@@ -419,71 +425,105 @@ void DecodedStream::StopPlayback()
     return;
   }
 
   mStartTime.reset();
   DisconnectListener();
 
   // Clear mData immediately when this playback session ends so we won't
   // send data to the wrong stream in SendData() in next playback session.
-  DecodedStreamData* data = mData.release();
-  // mData is not yet created on the main thread.
-  if (!data) {
+  DestroyData(Move(mData));
+}
+
+void
+DecodedStream::DestroyData(UniquePtr<DecodedStreamData> aData)
+{
+  AssertOwnerThread();
+  ReentrantMonitorAutoEnter mon(GetReentrantMonitor());
+
+  if (!aData) {
     return;
   }
 
+  DecodedStreamData* data = aData.release();
   nsRefPtr<DecodedStream> self = this;
   nsCOMPtr<nsIRunnable> r = NS_NewRunnableFunction([=] () {
     self->mOutputStreamManager.Disconnect();
     delete data;
   });
   AbstractThread::MainThread()->Dispatch(r.forget());
 }
 
 void
 DecodedStream::CreateData(MozPromiseHolder<GenericPromise>&& aPromise)
 {
   MOZ_ASSERT(NS_IsMainThread());
   ReentrantMonitorAutoEnter mon(GetReentrantMonitor());
-  MOZ_ASSERT(!mData, "Already created.");
 
   // No need to create a source stream when there are no output streams. This
   // happens when RemoveOutput() is called immediately after StartPlayback().
-  // We also bail out when the playback session has ended. This happens when
-  // StopPlayback() is called immediately after StartPlayback().
-  if (!mOutputStreamManager.Graph() || mStartTime.isNothing()) {
+  // Also we don't create a source stream when MDSM has begun shutdown.
+  if (!mOutputStreamManager.Graph() || mShuttingDown) {
     // Resolve the promise to indicate the end of playback.
     aPromise.Resolve(true, __func__);
     return;
   }
 
   auto source = mOutputStreamManager.Graph()->CreateSourceStream(nullptr);
-  mData.reset(new DecodedStreamData(source, mPlaying, Move(aPromise)));
-  mOutputStreamManager.Connect(mData->mStream);
+  auto data = new DecodedStreamData(source, Move(aPromise));
+  mOutputStreamManager.Connect(data->mStream);
 
-  // Start to send data to the stream immediately
-  nsRefPtr<DecodedStream> self = this;
-  nsCOMPtr<nsIRunnable> r = NS_NewRunnableFunction([=] () {
-    ReentrantMonitorAutoEnter mon(self->GetReentrantMonitor());
-    // Don't send data if playback has ended.
-    if (self->mStartTime.isSome()) {
-      self->SendData();
+  class R : public nsRunnable {
+    typedef void(DecodedStream::*Method)(UniquePtr<DecodedStreamData>);
+  public:
+    R(DecodedStream* aThis, Method aMethod, DecodedStreamData* aData)
+      : mThis(aThis), mMethod(aMethod), mData(aData) {}
+    NS_IMETHOD Run() override
+    {
+      (mThis->*mMethod)(Move(mData));
+      return NS_OK;
     }
-  });
-  // Don't assert success because the owner thread might have begun shutdown
-  // while we are still dealing with jobs on the main thread.
-  mOwnerThread->Dispatch(r.forget(), AbstractThread::DontAssertDispatchSuccess);
+  private:
+    nsRefPtr<DecodedStream> mThis;
+    Method mMethod;
+    UniquePtr<DecodedStreamData> mData;
+  };
+
+  // Post a message to ensure |mData| is only updated on the worker thread.
+  // Note this must be done before MDSM's shutdown since dispatch could fail
+  // when the worker thread is shut down.
+  nsCOMPtr<nsIRunnable> r = new R(this, &DecodedStream::OnDataCreated, data);
+  mOwnerThread->Dispatch(r.forget());
 }
 
 bool
 DecodedStream::HasConsumers() const
 {
   return !mOutputStreamManager.IsEmpty();
 }
 
+void
+DecodedStream::OnDataCreated(UniquePtr<DecodedStreamData> aData)
+{
+  AssertOwnerThread();
+  ReentrantMonitorAutoEnter mon(GetReentrantMonitor());
+  MOZ_ASSERT(!mData, "Already created.");
+
+  // Start to send data to the stream immediately
+  if (mStartTime.isSome()) {
+    aData->SetPlaying(mPlaying);
+    mData = Move(aData);
+    SendData();
+    return;
+  }
+
+  // Playback has ended. Destroy aData which is not needed anymore.
+  DestroyData(Move(aData));
+}
+
 ReentrantMonitor&
 DecodedStream::GetReentrantMonitor() const
 {
   return mMonitor;
 }
 
 void
 DecodedStream::AddOutput(ProcessedMediaStream* aStream, bool aFinishWhenEnded)
--- a/dom/media/DecodedStream.h
+++ b/dom/media/DecodedStream.h
@@ -101,16 +101,18 @@ private:
 
 class DecodedStream {
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(DecodedStream);
 public:
   DecodedStream(AbstractThread* aOwnerThread,
                 MediaQueue<MediaData>& aAudioQueue,
                 MediaQueue<MediaData>& aVideoQueue);
 
+  void Shutdown();
+
   // Mimic MDSM::StartAudioThread.
   // Must be called before any calls to SendData().
   //
   // Return a promise which will be resolved when the stream is finished
   // or rejected if any error.
   nsRefPtr<GenericPromise> StartPlayback(int64_t aStartTime,
                                          const MediaInfo& aInfo);
   // Mimic MDSM::StopAudioThread.
@@ -129,44 +131,55 @@ public:
   bool HasConsumers() const;
 
 protected:
   virtual ~DecodedStream();
 
 private:
   ReentrantMonitor& GetReentrantMonitor() const;
   void CreateData(MozPromiseHolder<GenericPromise>&& aPromise);
+  void DestroyData(UniquePtr<DecodedStreamData> aData);
+  void OnDataCreated(UniquePtr<DecodedStreamData> aData);
   void InitTracks();
   void AdvanceTracks();
   void SendAudio(double aVolume, bool aIsSameOrigin);
   void SendVideo(bool aIsSameOrigin);
   void SendData();
 
   void AssertOwnerThread() const {
     MOZ_ASSERT(mOwnerThread->IsCurrentThreadIn());
   }
 
   void ConnectListener();
   void DisconnectListener();
 
   const nsRefPtr<AbstractThread> mOwnerThread;
 
-  UniquePtr<DecodedStreamData> mData;
+  /*
+   * Main thread only members.
+   */
   // Data about MediaStreams that are being fed by the decoder.
   OutputStreamManager mOutputStreamManager;
+  // True if MDSM has begun shutdown.
+  bool mShuttingDown;
 
   // TODO: This is a temp solution to get rid of decoder monitor on the main
   // thread in MDSM::AddOutputStream and MDSM::RecreateDecodedStream as
   // required by bug 1146482. DecodedStream needs to release monitor before
   // calling back into MDSM functions in order to prevent deadlocks.
   //
   // Please move all capture-stream related code from MDSM into DecodedStream
   // and apply "dispatch + mirroring" to get rid of this monitor in the future.
   mutable ReentrantMonitor mMonitor;
 
+  /*
+   * Worker thread only members.
+   */
+  UniquePtr<DecodedStreamData> mData;
+
   bool mPlaying;
   double mVolume;
   bool mSameOrigin;
 
   Maybe<int64_t> mStartTime;
   MediaInfo mInfo;
 
   MediaQueue<MediaData>& mAudioQueue;
--- a/dom/media/MediaDecoderStateMachine.h
+++ b/dom/media/MediaDecoderStateMachine.h
@@ -166,16 +166,17 @@ private:
   void DispatchAudioCaptured();
   void DispatchAudioUncaptured();
 
   void Shutdown();
 public:
 
   void DispatchShutdown()
   {
+    mDecodedStream->Shutdown();
     nsCOMPtr<nsIRunnable> runnable =
       NS_NewRunnableMethod(this, &MediaDecoderStateMachine::Shutdown);
     OwnerThread()->Dispatch(runnable.forget());
   }
 
   void FinishShutdown();
 
   // Immutable after construction - may be called on any thread.