Bug 1072775 - Additional assertions for MediaStreamGraph/GraphDriver. r=roc, a=lmandel
authorRandell Jesup <rjesup@jesup.org>
Sun, 28 Sep 2014 12:07:24 -0400
changeset 225440 c89cf06d1f3feb0e94c43d97715761c0956927a2
parent 225439 d5fcddc6091b74d2fb7449b46abdbb6041bcfa36
child 225441 6fc7dace7ec946e2e5c2a727a55feb7a70421a39
push id3979
push userraliiev@mozilla.com
push dateMon, 13 Oct 2014 16:35:44 +0000
treeherdermozilla-beta@30f2cc610691 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersroc, lmandel
bugs1072775
milestone34.0a2
Bug 1072775 - Additional assertions for MediaStreamGraph/GraphDriver. r=roc, a=lmandel
content/media/GraphDriver.cpp
content/media/GraphDriver.h
content/media/MediaStreamGraph.cpp
content/media/MediaStreamGraphImpl.h
--- a/content/media/GraphDriver.cpp
+++ b/content/media/GraphDriver.cpp
@@ -255,16 +255,17 @@ void
 ThreadedDriver::Stop()
 {
   NS_ASSERTION(NS_IsMainThread(), "Must be called on main thread");
   // mGraph's thread is not running so it's OK to do whatever here
   STREAM_LOG(PR_LOG_DEBUG, ("Stopping threads for MediaStreamGraph %p", this));
 
   if (mThread) {
     mThread->Shutdown();
+    mThread = nullptr;
   }
 }
 
 SystemClockDriver::SystemClockDriver(MediaStreamGraphImpl* aGraphImpl)
   : ThreadedDriver(aGraphImpl),
     mInitialTimeStamp(TimeStamp::Now()),
     mLastTimeStamp(TimeStamp::Now())
 {}
@@ -395,29 +396,35 @@ class MediaStreamGraphShutdownThreadRunn
 public:
   explicit MediaStreamGraphShutdownThreadRunnable2(nsIThread* aThread)
     : mThread(aThread)
   {
   }
   NS_IMETHOD Run()
   {
     MOZ_ASSERT(NS_IsMainThread());
+    MOZ_ASSERT(mThread);
+
     mThread->Shutdown();
+    mThread = nullptr;
     return NS_OK;
   }
 private:
-  nsRefPtr<nsIThread> mThread;
+  nsCOMPtr<nsIThread> mThread;
 };
 
 OfflineClockDriver::~OfflineClockDriver()
 {
   // transfer the ownership of mThread to the event
-  nsCOMPtr<nsIRunnable> event = new MediaStreamGraphShutdownThreadRunnable2(mThread);
-  mThread = nullptr;
-  NS_DispatchToMainThread(event);
+  // XXX should use .forget()/etc
+  if (mThread) {
+    nsCOMPtr<nsIRunnable> event = new MediaStreamGraphShutdownThreadRunnable2(mThread);
+    mThread = nullptr;
+    NS_DispatchToMainThread(event);
+  }
 }
 
 void
 OfflineClockDriver::GetIntervalForIteration(GraphTime& aFrom, GraphTime& aTo)
 {
   aFrom = mIterationStart = IterationEnd();
   aTo = mIterationEnd = IterationEnd() + mGraphImpl->MillisecondsToMediaTime(mSlice);
 
@@ -735,29 +742,26 @@ AudioCallbackDriver::StateCallback_s(cub
 /* static */ void
 AudioCallbackDriver::DeviceChangedCallback_s(void* aUser)
 {
   AudioCallbackDriver* driver = reinterpret_cast<AudioCallbackDriver*>(aUser);
   driver->DeviceChangedCallback();
 }
 
 bool AudioCallbackDriver::InCallback() {
-  MonitorAutoLock mon(mGraphImpl->GetMonitor());
   return mInCallback;
 }
 
 AudioCallbackDriver::AutoInCallback::AutoInCallback(AudioCallbackDriver* aDriver)
   : mDriver(aDriver)
 {
-  MonitorAutoLock mon(mDriver->mGraphImpl->GetMonitor());
   mDriver->mInCallback = true;
 }
 
 AudioCallbackDriver::AutoInCallback::~AutoInCallback() {
-  MonitorAutoLock mon(mDriver->mGraphImpl->GetMonitor());
   mDriver->mInCallback = false;
 }
 
 long
 AudioCallbackDriver::DataCallback(AudioDataValue* aBuffer, long aFrames)
 {
   bool stillProcessing;
 
--- a/content/media/GraphDriver.h
+++ b/content/media/GraphDriver.h
@@ -6,16 +6,17 @@
 #ifndef GRAPHDRIVER_H_
 #define GRAPHDRIVER_H_
 
 #include "nsAutoPtr.h"
 #include "nsAutoRef.h"
 #include "AudioBufferUtils.h"
 #include "AudioMixer.h"
 #include "AudioSegment.h"
+#include "mozilla/Atomics.h"
 
 struct cubeb_stream;
 
 namespace mozilla {
 
 
 /**
  * Assume we can run an iteration of the MediaStreamGraph loop in this much time
@@ -187,16 +188,18 @@ public:
    * Same thing, but not locked.
    */
   void EnsureNextIterationLocked();
 
   MediaStreamGraphImpl* GraphImpl() {
     return mGraphImpl;
   }
 
+  virtual bool OnThread() = 0;
+
 protected:
   // Time of the start of this graph iteration.
   GraphTime mIterationStart;
   // Time of the end of this graph iteration.
   GraphTime mIterationEnd;
   // Time, in the future, for which blocking has been computed.
   GraphTime mStateComputedTime;
   GraphTime mNextStateComputedTime;
@@ -251,16 +254,19 @@ public:
    * Runs main control loop on the graph thread. Normally a single invocation
    * of this runs for the entire lifetime of the graph thread.
    */
   void RunThread();
   friend class MediaStreamGraphInitThreadRunnable;
   uint32_t IterationDuration() {
     return MEDIA_GRAPH_TARGET_PERIOD_MS;
   }
+
+  virtual bool OnThread() MOZ_OVERRIDE { return !mThread || NS_GetCurrentThread() == mThread; }
+
 protected:
   nsCOMPtr<nsIThread> mThread;
 };
 
 /**
  * A SystemClockDriver drives a MediaStreamGraph using a system clock, and waits
  * using a monitor, between each iteration.
  */
@@ -373,16 +379,18 @@ public:
     return this;
   }
 
   /**
    * Whether the audio callback is processing. This is for asserting only.
    */
   bool InCallback();
 
+  virtual bool OnThread() MOZ_OVERRIDE { return !mStarted || InCallback(); }
+
   /* Whether the underlying cubeb stream has been started. See comment for
    * mStarted for details. */
   bool IsStarted();
 
   /* Tell the driver whether this process is using a microphone or not. This is
    * thread safe. */
   void SetMicrophoneActive(bool aActive);
 private:
@@ -438,18 +446,17 @@ private:
     ~AutoInCallback();
     AudioCallbackDriver* mDriver;
   };
 
   /* Thread for off-main-thread initialization and
    * shutdown of the audio stream. */
   nsCOMPtr<nsIThread> mInitShutdownThread;
   dom::AudioChannel mAudioChannel;
-  /* This can only be accessed with the graph's monitor held. */
-  bool mInCallback;
+  Atomic<bool> mInCallback;
   /* A thread has been created to be able to pause and restart the audio thread,
    * but has not done so yet. This indicates that the callback should return
    * early */
   bool mPauseRequested;
   /**
    * True if microphone is being used by this process. This is synchronized by
    * the graph's monitor. */
   bool mMicrophoneActive;
--- a/content/media/MediaStreamGraph.cpp
+++ b/content/media/MediaStreamGraph.cpp
@@ -1481,21 +1481,28 @@ public:
   {}
   NS_IMETHOD Run()
   {
     NS_ASSERTION(mGraph->mDetectedNotRunning,
                  "We should know the graph thread control loop isn't running!");
 
     LIFECYCLE_LOG("Shutting down graph %p", mGraph.get());
 
-    if (mGraph->CurrentDriver()->AsAudioCallbackDriver()) {
-      MOZ_ASSERT(!mGraph->CurrentDriver()->AsAudioCallbackDriver()->InCallback());
+    // We've asserted the graph isn't running.  Use mDriver instead of CurrentDriver
+    // to avoid thread-safety checks
+#if 0 // AudioCallbackDrivers are released asynchronously anyways
+    // XXX a better test would be have setting mDetectedNotRunning make sure
+    // any current callback has finished and block future ones -- or just
+    // handle it all in Shutdown()!
+    if (mGraph->mDriver->AsAudioCallbackDriver()) {
+      MOZ_ASSERT(!mGraph->mDriver->AsAudioCallbackDriver()->InCallback());
     }
+#endif
 
-    mGraph->CurrentDriver()->Shutdown();
+    mGraph->mDriver->Shutdown();
 
     // mGraph's thread is not running so it's OK to do whatever here
     if (mGraph->IsEmpty()) {
       // mGraph is no longer needed, so delete it.
       mGraph->Destroy();
     } else {
       // The graph is not empty.  We must be in a forced shutdown, or a
       // non-realtime graph that has finished processing.  Some later
@@ -1713,16 +1720,17 @@ MediaStreamGraphImpl::RunInStableState(b
   }
 
 #ifdef DEBUG
   mCanRunMessagesSynchronously = mDetectedNotRunning &&
     mLifecycleState >= LIFECYCLE_WAITING_FOR_THREAD_SHUTDOWN;
 #endif
 }
 
+
 static NS_DEFINE_CID(kAppShellCID, NS_APPSHELL_CID);
 
 void
 MediaStreamGraphImpl::EnsureRunInStableState()
 {
   NS_ASSERTION(NS_IsMainThread(), "main thread only");
 
   if (mPostedRunInStableState)
--- a/content/media/MediaStreamGraphImpl.h
+++ b/content/media/MediaStreamGraphImpl.h
@@ -145,16 +145,22 @@ public:
   void ShutdownThreads();
 
   /**
    * Called before the thread runs.
    */
   void Init();
   // The following methods run on the graph thread (or possibly the main thread if
   // mLifecycleState > LIFECYCLE_RUNNING)
+  void AssertOnGraphThreadOrNotRunning() {
+    // either we're on the right thread (and calling CurrentDriver() is safe),
+    // or we're going to assert anyways, so don't cross-check CurrentDriver
+    MOZ_ASSERT(mDriver->OnThread() ||
+               (mLifecycleState > LIFECYCLE_RUNNING && NS_IsMainThread()));
+  }
   /*
    * This does the actual iteration: Message processing, MediaStream ordering,
    * blocking computation and processing.
    */
   void DoIteration();
 
   bool OneIteration(GraphTime aFrom, GraphTime aTo,
                     GraphTime aStateFrom, GraphTime aStateEnd);
@@ -410,26 +416,33 @@ public:
   /**
    * Signal to the graph that the thread has paused indefinitly,
    * or resumed.
    */
   void PausedIndefinitly();
   void ResumedFromPaused();
 
   GraphDriver* CurrentDriver() {
+#ifdef DEBUG
+    // #ifdef since we're not wrapping it all in MOZ_ASSERT()
+    if (!mDriver->OnThread()) {
+      mMonitor.AssertCurrentThreadOwns();
+    }
+#endif
     return mDriver;
   }
 
   /**
    * Effectively set the new driver, while we are switching.
    * It is only safe to call this at the very end of an iteration, when there
    * has been a SwitchAtNextIteration call during the iteration. The driver
    * should return and pass the control to the new driver shortly after.
    */
   void SetCurrentDriver(GraphDriver* aDriver) {
+    MOZ_ASSERT(mDriver->OnThread());
     mDriver = aDriver;
   }
 
   Monitor& GetMonitor() {
     return mMonitor;
   }
 
   // Data members