Backed out 5 changesets (bug 1625372) for assertion failures on GraphDriver.cpp. CLOSED TREE

Backed out changeset b9feee4f447c (bug 1625372)
Backed out changeset d86f066bd68b (bug 1625372)
Backed out changeset 7a38398623f8 (bug 1625372)
Backed out changeset 6e9928fb37d1 (bug 1625372)
Backed out changeset ca93e9921c05 (bug 1625372)

dom/base/nsGlobalWindowInner.cpp

@@ -1135,7 +1135,7 @@ void nsGlobalWindowInner::FreeInnerObjects() {
   NotifyWindowIDDestroyed("inner-window-destroyed");
 
   for (uint32_t i = 0; i < mAudioContexts.Length(); ++i) {
-    mAudioContexts[i]->OnWindowDestroy();
+    mAudioContexts[i]->Shutdown();
   }
   mAudioContexts.Clear();
 

dom/media/GraphDriver.cpp

@@ -7,6 +7,7 @@
 #include "mozilla/dom/AudioContext.h"
 #include "mozilla/dom/AudioDeviceInfo.h"
 #include "mozilla/dom/BaseAudioContextBinding.h"
+#include "mozilla/dom/WorkletThread.h"
 #include "mozilla/SchedulerGroup.h"
 #include "mozilla/SharedThreadPool.h"
 #include "mozilla/ClearOnShutdown.h"
@@ -205,6 +206,7 @@ void ThreadedDriver::RunThread() {
 
     if (result.IsStop()) {
       // Signal that we're done stopping.
+      dom::WorkletThread::DeleteCycleCollectedJSContext();
       result.Stopped();
       break;
     }

dom/media/GraphRunner.cpp

@@ -9,8 +9,8 @@
 #include "GraphDriver.h"
 #include "MediaTrackGraph.h"
 #include "MediaTrackGraphImpl.h"
+#include "mozilla/dom/WorkletThread.h"
 #include "nsISupportsImpl.h"
-#include "nsISupportsPriority.h"
 #include "prthread.h"
 #include "Tracing.h"
 #include "audio_thread_priority.h"
@@ -123,6 +123,8 @@ NS_IMETHODIMP GraphRunner::Run() {
     atp_demote_current_thread_from_real_time(handle);
   }
 
+  dom::WorkletThread::DeleteCycleCollectedJSContext();
+
   return NS_OK;
 }
 

dom/media/MediaTrackGraph.cpp

@@ -22,7 +22,6 @@
 #include "AudioNodeExternalInputTrack.h"
 #include "MediaTrackListener.h"
 #include "mozilla/dom/BaseAudioContextBinding.h"
-#include "mozilla/dom/WorkletThread.h"
 #include "mozilla/media/MediaUtils.h"
 #include <algorithm>
 #include "GeckoProfiler.h"
@@ -1337,29 +1336,25 @@ void MediaTrackGraphImpl::Process(AudioMixer* aMixer) {
 
 bool MediaTrackGraphImpl::UpdateMainThreadState() {
   MOZ_ASSERT(OnGraphThread());
-  if (mForceShutDownReceived) {
+  if (mForceShutDown) {
     for (MediaTrack* track : AllTracks()) {
       track->NotifyForcedShutdown();
     }
   }
-  {
+
-    MonitorAutoLock lock(mMonitor);
+  MonitorAutoLock lock(mMonitor);
-    bool finalUpdate =
+  bool finalUpdate =
-        mForceShutDownReceived || (IsEmpty() && mBackMessageQueue.IsEmpty());
+      mForceShutDown || (IsEmpty() && mBackMessageQueue.IsEmpty());
-    PrepareUpdatesToMainThreadState(finalUpdate);
+  PrepareUpdatesToMainThreadState(finalUpdate);
-    if (!finalUpdate) {
+  if (finalUpdate) {
-      SwapMessageQueues();
+    // Enter shutdown mode when this iteration is completed.
-      return true;
+    // No need to Destroy tracks here. The main-thread owner of each
-    }
+    // track is responsible for calling Destroy on them.
-    // The JSContext will not be used again.
+    return false;
-    // Clear main thread access while under monitor.
-    mJSContext = nullptr;
   }
-  dom::WorkletThread::DeleteCycleCollectedJSContext();
+
-  // Enter shutdown mode when this iteration is completed.
+  SwapMessageQueues();
-  // No need to Destroy tracks here. The main-thread owner of each
+  return true;
-  // track is responsible for calling Destroy on them.
-  return false;
 }
 
 auto MediaTrackGraphImpl::OneIteration(GraphTime aStateEnd,
@@ -1474,7 +1469,7 @@ void MediaTrackGraphImpl::ForceShutDown() {
    public:
     explicit Message(MediaTrackGraphImpl* aGraph)
         : ControlMessage(nullptr), mGraph(aGraph) {}
-    void Run() override { mGraph->mForceShutDownReceived = true; }
+    void Run() override { mGraph->mForceShutDown = true; }
     // The graph owns this message.
     MediaTrackGraphImpl* MOZ_NON_OWNING_REF mGraph;
   };
@@ -1483,7 +1478,6 @@ void MediaTrackGraphImpl::ForceShutDown() {
     // If both the track and port counts are zero, the regular shutdown
     // sequence will progress shortly to shutdown threads and destroy the graph.
     AppendMessage(MakeUnique<Message>(this));
-    InterruptJS();
   }
 }
 
@@ -1618,10 +1612,11 @@ class MediaTrackGraphShutDownRunnable : public Runnable {
       // mGraph is no longer needed, so delete it.
       mGraph->Destroy();
     } else {
-      // The graph is not empty.  We must be in a forced shutdown.
+      // The graph is not empty.  We must be in a forced shutdown, either for
-      // Some later AppendMessage will detect that the graph has
+      // process shutdown or a non-realtime graph that has finished
+      // processing. Some later AppendMessage will detect that the graph has
       // been emptied, and delete it.
-      NS_ASSERTION(mGraph->mForceShutDownReceived, "Not in forced shutdown?");
+      NS_ASSERTION(mGraph->mForceShutDown, "Not in forced shutdown?");
       mGraph->LifecycleStateRef() =
           MediaTrackGraphImpl::LIFECYCLE_WAITING_FOR_TRACK_DESTRUCTION;
     }
@@ -1734,7 +1729,7 @@ void MediaTrackGraphImpl::RunInStableState(bool aSourceIsMTG) {
       // If this MediaTrackGraph has entered regular (non-forced) shutdown it
       // is not able to process any more messages. Those messages being added to
       // the graph in the first place is an error.
-      MOZ_DIAGNOSTIC_ASSERT(mForceShutDownReceived ||
+      MOZ_DIAGNOSTIC_ASSERT(mForceShutDown ||
                             LifecycleStateRef() <
                                 LIFECYCLE_WAITING_FOR_MAIN_THREAD_CLEANUP);
     }
@@ -1764,7 +1759,7 @@ void MediaTrackGraphImpl::RunInStableState(bool aSourceIsMTG) {
     }
 
     if (LifecycleStateRef() == LIFECYCLE_WAITING_FOR_MAIN_THREAD_CLEANUP &&
-        mForceShutDownReceived) {
+        mForceShutDown) {
       // Defer calls to RunDuringShutdown() to happen while mMonitor is not
       // held.
       for (uint32_t i = 0; i < mBackMessageQueue.Length(); ++i) {
@@ -2962,6 +2957,7 @@ MediaTrackGraphImpl::MediaTrackGraphImpl(
       mOutputDeviceID(aOutputDeviceID),
       mMonitor("MediaTrackGraphImpl"),
       mLifecycleState(LIFECYCLE_THREAD_NOT_STARTED),
+      mForceShutDown(false),
       mPostedRunInStableStateEvent(false),
       mGraphDriverRunning(false),
       mPostedRunInStableState(false),
@@ -3128,10 +3124,12 @@ MediaTrackGraph* MediaTrackGraph::CreateNonRealtimeInstance(
   return graph;
 }
 
-void MediaTrackGraph::ForceShutDown() {
+void MediaTrackGraph::DestroyNonRealtimeInstance(MediaTrackGraph* aGraph) {
   MOZ_ASSERT(NS_IsMainThread(), "Main thread only");
+  MOZ_ASSERT(aGraph->IsNonRealtime(),
+             "Should not destroy the global graph here");
 
-  MediaTrackGraphImpl* graph = static_cast<MediaTrackGraphImpl*>(this);
+  MediaTrackGraphImpl* graph = static_cast<MediaTrackGraphImpl*>(aGraph);
 
   graph->ForceShutDown();
 }
@@ -3318,9 +3316,6 @@ void MediaTrackGraphImpl::RemoveTrack(MediaTrack* aTrack) {
         break;
       }
     }
-    // The graph thread will shut itself down soon, but won't be able to do
-    // that if JS continues to run.
-    InterruptJS();
   }
 }
 
@@ -3694,38 +3689,6 @@ void MediaTrackGraph::StartNonRealtimeProcessing(uint32_t aTicksToProcess) {
   graph->AppendMessage(MakeUnique<Message>(graph, aTicksToProcess));
 }
 
-void MediaTrackGraphImpl::InterruptJS() {
-  MonitorAutoLock lock(mMonitor);
-  mInterruptJSCalled = true;
-  if (mJSContext) {
-    JS_RequestInterruptCallback(mJSContext);
-  }
-}
-
-static bool InterruptCallback(JSContext* aCx) {
-  // Interrupt future calls also.
-  JS_RequestInterruptCallback(aCx);
-  // Stop execution.
-  return false;
-}
-
-void MediaTrackGraph::NotifyJSContext(JSContext* aCx) {
-  MOZ_ASSERT(OnGraphThread());
-  MOZ_ASSERT(aCx);
-
-  auto* impl = static_cast<MediaTrackGraphImpl*>(this);
-  if (impl->mJSContext) {
-    MOZ_ASSERT(impl->mJSContext == aCx);
-    return;
-  }
-  JS_AddInterruptCallback(aCx, InterruptCallback);
-  MonitorAutoLock lock(impl->mMonitor);
-  impl->mJSContext = aCx;
-  if (impl->mInterruptJSCalled) {
-    JS_RequestInterruptCallback(aCx);
-  }
-}
-
 void ProcessedMediaTrack::AddInput(MediaInputPort* aPort) {
   MediaTrack* t = aPort->GetSource();
   if (!t->IsSuspended()) {

dom/media/MediaTrackGraph.h

@@ -1026,7 +1026,7 @@ class MediaTrackGraph {
   AbstractThread* AbstractMainThread();
 
   // Idempotent
-  void ForceShutDown();
+  static void DestroyNonRealtimeInstance(MediaTrackGraph* aGraph);
 
   virtual nsresult OpenAudioInput(CubebUtils::AudioDeviceID aID,
                                   AudioDataListener* aListener) = 0;
@@ -1096,12 +1096,6 @@ class MediaTrackGraph {
    */
   void StartNonRealtimeProcessing(uint32_t aTicksToProcess);
 
-  /**
-   * NotifyJSContext() is called on the graph thread before content script
-   * runs.
-   */
-  void NotifyJSContext(JSContext* aCx);
-
   /**
    * Media graph thread only.
    * Dispatches a runnable that will run on the main thread after all

dom/media/MediaTrackGraphImpl.h

@@ -174,8 +174,7 @@ class MediaTrackGraphImpl : public MediaTrackGraph,
    * Make this MediaTrackGraph enter forced-shutdown state. This state
    * will be noticed by the media graph thread, which will shut down all tracks
    * and other state controlled by the media graph thread.
-   * This is called during application shutdown, and on document unload if an
+   * This is called during application shutdown.
-   * AudioContext is using the graph.
    */
   void ForceShutDown();
 
@@ -624,12 +623,6 @@ class MediaTrackGraphImpl : public MediaTrackGraph,
 
   Watchable<GraphTime>& CurrentTime() override;
 
-  /**
-   * Interrupt any JS running on the graph thread.
-   * Called on the main thread when shutting down the graph.
-   */
-  void InterruptJS();
-
   class TrackSet {
    public:
     class iterator {
@@ -842,13 +835,13 @@ class MediaTrackGraphImpl : public MediaTrackGraph,
    * creation after this point will create a new graph. An async event is
    * dispatched to Shutdown() the graph's threads and then delete the graph
    * object.
-   * 2) Forced shutdown at application shutdown, completion of a non-realtime
+   * 2) Forced shutdown at application shutdown, or completion of a
-   * graph, or document unload. A flag is set, RunThread() detects the flag
+   * non-realtime graph. A flag is set, RunThread() detects the flag and
-   * and exits, the next RunInStableState() detects the flag, and dispatches
+   * exits, the next RunInStableState() detects the flag, and dispatches the
-   * the async event to Shutdown() the graph's threads. However the graph
+   * async event to Shutdown() the graph's threads. However the graph object
-   * object is not deleted. New messages for the graph are processed
+   * is not deleted. New messages for the graph are processed synchronously on
-   * synchronously on the main thread if necessary. When the last track is
+   * the main thread if necessary. When the last track is destroyed, the
-   * destroyed, the graph object is deleted.
+   * graph object is deleted.
    *
    * This should be kept in sync with the LifecycleState_str array in
    * MediaTrackGraph.cpp
@@ -903,21 +896,13 @@ class MediaTrackGraphImpl : public MediaTrackGraph,
   }
 
   /**
-   * True once the graph thread has received the message from ForceShutDown().
+   * True when we need to do a forced shutdown, during application shutdown or
-   * This is checked in the decision to shut down the
+   * when shutting down a non-realtime graph.
-   * graph thread so that control messages dispatched before forced shutdown
-   * are processed on the graph thread.
    * Only set on the graph thread.
    * Can be read safely on the thread currently owning the graph, as indicated
    * by mLifecycleState.
    */
-  bool mForceShutDownReceived = false;
+  bool mForceShutDown;
-  /**
-   * true when InterruptJS() has been called, because shutdown (normal or
-   * forced) has commenced.  Set on the main thread under mMonitor and read on
-   * the graph thread under mMonitor.
-   **/
-  bool mInterruptJSCalled = false;
 
   /**
    * Remove this blocker to unblock shutdown.
@@ -932,14 +917,6 @@ class MediaTrackGraphImpl : public MediaTrackGraph,
    */
   bool mPostedRunInStableStateEvent;
 
-  /**
-   * The JSContext of the graph thread.  Set under mMonitor on only the graph
-   * or GraphRunner thread.  Once set this does not change until reset when
-   * the thread is about to exit.  Read under mMonitor on the main thread to
-   * interrupt running JS for forced shutdown.
-   **/
-  JSContext* mJSContext = nullptr;
-
   // Main thread only
 
   /**

dom/media/test/crashtests/audioworklet-iloop-1.html

@@ -1,24 +0,0 @@
-<!DOCTYPE html>
-<html class="reftest-wait">
-<head>
-  <title>Bug 1625372: JS infinite loops on AudioWorklet thread</title>
-  <script id="processor" type="worklet">
-registerProcessor("iloop", class extends AudioWorkletProcessor {
-  constructor() { while (true) {} }
-});
-  </script>
-  <script>
-const realtime = new AudioContext();
-const offline = new OfflineAudioContext({length: 1, sampleRate: 16384});
-const url = URL.createObjectURL(
-  new Blob([document.getElementById("processor").innerText]),
-  {type: "application/javascript"});
-Promise.all([
-  realtime.audioWorklet.addModule(url).then(
-    () => new AudioWorkletNode(realtime, "iloop")),
-  offline.audioWorklet.addModule(url).then(
-    () => new AudioWorkletNode(offline, "iloop"))
-]).then(() => document.documentElement.removeAttribute("class"));
-  </script>
-</head>
-</html>

dom/media/test/crashtests/crashtests.list

@@ -101,7 +101,6 @@ load 1450845.html
 load disconnect-wrong-destination.html
 load analyser-channels-1.html
 skip-if(verify&&isDebugBuild&&gtkWidget) load audiocontext-double-suspend.html
-skip-if(Android) load audioworklet-iloop-1.html # bug 1637491
 load buffer-source-duration-1.html
 skip-if(verify&&isDebugBuild&&gtkWidget) load buffer-source-ended-1.html
 load buffer-source-resampling-start-1.html

dom/media/webaudio/AudioContext.cpp

@@ -741,7 +741,7 @@ nsISerialEventTarget* AudioContext::GetMainThread() const {
 
 void AudioContext::DisconnectFromOwner() {
   mIsDisconnecting = true;
-  OnWindowDestroy();
+  Shutdown();
   DOMEventTargetHelper::DisconnectFromOwner();
 }
 
@@ -759,7 +759,7 @@ void AudioContext::BindToOwner(nsIGlobalObject* aNew) {
   }
 }
 
-void AudioContext::OnWindowDestroy() {
+void AudioContext::Shutdown() {
   // Avoid resend the Telemetry data.
   if (!mIsShutDown) {
     MaybeUpdateAutoplayTelemetryWhenShutdown();
@@ -788,18 +788,9 @@ void AudioContext::OnWindowDestroy() {
   // PBrowser::Destroy() message before xpcom shutdown begins.
   ShutdownWorklet();
 
-  if (mDestination) {
+  // For offline contexts, we can destroy the MediaTrackGraph at this point.
-    // We can destroy the MediaTrackGraph at this point.
+  if (mIsOffline && mDestination) {
-    // Although there may be other clients using the graph, this graph is used
+    mDestination->OfflineShutdown();
-    // only for clients in the same window and this window is going away.
-    // This will also interrupt any worklet script still running on the graph
-    // thread.
-    Graph()->ForceShutDown();
-    // AudioDestinationNodes on rendering offline contexts have a
-    // self-reference which needs removal.
-    if (mIsOffline) {
-      mDestination->OfflineShutdown();
-    }
   }
 }
 

dom/media/webaudio/AudioContext.h

@@ -152,7 +152,7 @@ class AudioContext final : public DOMEventTargetHelper,
   virtual void DisconnectFromOwner() override;
   virtual void BindToOwner(nsIGlobalObject* aNew) override;
 
-  void OnWindowDestroy();  // idempotent
+  void Shutdown();  // idempotent
 
   JSObject* WrapObject(JSContext* aCx,
                        JS::Handle<JSObject*> aGivenProto) override;

dom/media/webaudio/AudioDestinationNode.cpp

@@ -414,6 +414,10 @@ void AudioDestinationNode::DestroyMediaTrack() {
   Context()->ShutdownWorklet();
 
   mTrack->RemoveMainThreadListener(this);
+  MediaTrackGraph* graph = mTrack->Graph();
+  if (graph->IsNonRealtime()) {
+    MediaTrackGraph::DestroyNonRealtimeInstance(graph);
+  }
   AudioNode::DestroyMediaTrack();
 }
 
@@ -498,7 +502,10 @@ void AudioDestinationNode::OfflineShutdown() {
   MOZ_ASSERT(Context() && Context()->IsOffline(),
              "Should only be called on a valid OfflineAudioContext");
 
-  mOfflineRenderingRef.Drop(this);
+  if (mTrack) {
+    MediaTrackGraph::DestroyNonRealtimeInstance(mTrack->Graph());
+    mOfflineRenderingRef.Drop(this);
+  }
 }
 
 JSObject* AudioDestinationNode::WrapObject(JSContext* aCx,

dom/media/webaudio/AudioWorkletGlobalScope.cpp

@@ -40,10 +40,6 @@ AudioWorkletGlobalScope::AudioWorkletGlobalScope(AudioWorkletImpl* aImpl)
 
 bool AudioWorkletGlobalScope::WrapGlobalObject(
     JSContext* aCx, JS::MutableHandle<JSObject*> aReflector) {
-  // |this| is being exposed to JS and content script will soon be running.
-  // The graph needs a handle on the JSContext so it can interrupt JS.
-  mImpl->DestinationTrack()->Graph()->NotifyJSContext(aCx);
-
   JS::RealmOptions options;
 
   // The SharedArrayBuffer global constructor property should not be present in

dom/worklet/WorkletThread.cpp

@@ -354,6 +354,7 @@ void WorkletThread::EnsureCycleCollectedJSContext(JSRuntime* aParentRuntime) {
   // FIXME: JSSettings
   // FIXME: JS_SetSecurityCallbacks
   // FIXME: JS::SetAsyncTaskCallbacks
+  // FIXME: JS_AddInterruptCallback
   // FIXME: JS::SetCTypesActivityCallback
   // FIXME: JS_SetGCZeal