bug 1538630 Check a predicate when waiting on condition variables in GraphRunner r=pehrsons

so as to respond appropriately to spurious wakeups.

Checking the predicate before the first wait in Run() makes the mStarted
notification unnecessary.

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

--HG--
extra : moz-landing-system : lando

dom/media/GraphRunner.cpp

@@ -26,8 +26,7 @@ GraphRunner::GraphRunner(MediaStreamGraphImpl* aGraph)
       mGraph(aGraph),
       mStateEnd(0),
       mStillProcessing(true),
-      mShutdown(false),
-      mStarted(false),
+      mThreadState(ThreadState::Wait),
       // Note that mThread needs to be initialized last, as it may pre-empt the
       // thread running this ctor and enter Run() with uninitialized members.
       mThread(PR_CreateThread(PR_SYSTEM_THREAD, &Start, this,
@@ -38,18 +37,14 @@ GraphRunner::GraphRunner(MediaStreamGraphImpl* aGraph)
 
 GraphRunner::~GraphRunner() {
   MOZ_COUNT_DTOR(GraphRunner);
-#ifdef DEBUG
-  {
-    MonitorAutoLock lock(mMonitor);
-    MOZ_ASSERT(mShutdown);
-  }
-#endif
+  MOZ_ASSERT(mThreadState == ThreadState::Shutdown);
   PR_JoinThread(mThread);
 }
 
 void GraphRunner::Shutdown() {
   MonitorAutoLock lock(mMonitor);
-  mShutdown = true;
+  MOZ_ASSERT(mThreadState == ThreadState::Wait);
+  mThreadState = ThreadState::Shutdown;
   mMonitor.Notify();
 }
 
@@ -57,14 +52,9 @@ bool GraphRunner::OneIteration(GraphTime aStateEnd) {
   TRACE_AUDIO_CALLBACK();
 
   MonitorAutoLock lock(mMonitor);
-  MOZ_ASSERT(!mShutdown);
+  MOZ_ASSERT(mThreadState == ThreadState::Wait);
   mStateEnd = aStateEnd;
 
-  if (!mStarted) {
-    mMonitor.Wait();
-    MOZ_ASSERT(mStarted);
-  }
-
 #ifdef DEBUG
   if (auto audioDriver = mGraph->CurrentDriver()->AsAudioCallbackDriver()) {
     mAudioDriverThreadId = audioDriver->ThreadId();
@@ -75,9 +65,13 @@ bool GraphRunner::OneIteration(GraphTime aStateEnd) {
     MOZ_CRASH("Unknown GraphDriver");
   }
 #endif
-
-  mMonitor.Notify();  // Signal that mStateEnd was updated
-  mMonitor.Wait();    // Wait for mStillProcessing to update
+  // Signal that mStateEnd was updated
+  mThreadState = ThreadState::Run;
+  mMonitor.Notify();
+  // Wait for mStillProcessing to update
+  do {
+    mMonitor.Wait();
+  } while (mThreadState == ThreadState::Run);
 
 #ifdef DEBUG
   mAudioDriverThreadId = std::thread::id();
@@ -90,17 +84,18 @@ bool GraphRunner::OneIteration(GraphTime aStateEnd) {
 void GraphRunner::Run() {
   PR_SetCurrentThreadName("GraphRunner");
   MonitorAutoLock lock(mMonitor);
-  mStarted = true;
-  mMonitor.Notify();  // Signal that mStarted was set, in case the audio
-                      // callback is already waiting for us
   while (true) {
-    mMonitor.Wait();  // Wait for mStateEnd or mShutdown to update
-    if (mShutdown) {
+    while (mThreadState == ThreadState::Wait) {
+      mMonitor.Wait();  // Wait for mStateEnd to update or for shutdown
+    }
+    if (mThreadState == ThreadState::Shutdown) {
       break;
     }
     TRACE();
     mStillProcessing = mGraph->OneIterationImpl(mStateEnd);
-    mMonitor.Notify();  // Signal that mStillProcessing was updated
+    // Signal that mStillProcessing was updated
+    mThreadState = ThreadState::Wait;
+    mMonitor.Notify();
   }
 
   dom::WorkletThread::DeleteCycleCollectedJSContext();

dom/media/GraphRunner.h

@@ -65,11 +65,19 @@ class GraphRunner {
   GraphTime mStateEnd;
   // Reply from mGraph's OneIteration. Protected by mMonitor.
   bool mStillProcessing;
-  // True after Shutdown(). Protected by mMonitor.
-  bool mShutdown;
-  // True after mThread has started running and has entered its main loop.
-  // Protected by mMonitor.
-  bool mStarted;
+
+  enum class ThreadState {
+    Wait,      // Waiting for a message.  This is the initial state.
+               // A transition from Run back to Wait occurs on the runner
+               // thread after it processes as far as mStateEnd and sets
+               // mStillProcessing.
+    Run,       // Set on driver thread after each mStateEnd update.
+    Shutdown,  // Set when Shutdown() is called on main thread.
+  };
+  // Protected by mMonitor until set to Shutdown, after which this is not
+  // modified.
+  ThreadState mThreadState;
+
   // The thread running mGraph.
   PRThread* const mThread;