Bug 1631304 - Mark the tail dispatcher as unavailable outside of event dispatch. r=jya
authorBobby Holley <bobbyholley@gmail.com>
Tue, 28 Apr 2020 21:18:21 +0000
changeset 526568 05287026736e9ba13d27a419298ba62ad437faa6
parent 526567 bad4d84f41cae3804de1b494172630bd661dde7f
child 526569 658d78ebfd03ce9500c08a25a8a949450f6ae448
push id37358
push useropoprus@mozilla.com
push dateWed, 29 Apr 2020 03:05:14 +0000
treeherdermozilla-central@6bb8423186c1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjya
bugs1631304
milestone77.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 1631304 - Mark the tail dispatcher as unavailable outside of event dispatch. r=jya This is in preparation for running the tail dispatcher off nsIThreadObserver callbacks, which only work during regular event processing. Differential Revision: https://phabricator.services.mozilla.com/D72264
dom/html/HTMLMediaElement.cpp
xpcom/threads/AbstractThread.cpp
xpcom/threads/AbstractThread.h
xpcom/threads/StateWatching.h
xpcom/threads/TaskQueue.cpp
--- a/dom/html/HTMLMediaElement.cpp
+++ b/dom/html/HTMLMediaElement.cpp
@@ -5311,17 +5311,17 @@ void HTMLMediaElement::NotifyMediaStream
     if (VideoTracks()->SelectedIndex() == -1) {
       MOZ_ASSERT(!mSelectedVideoStreamTrack);
       videoTrack->SetEnabledInternal(true, dom::MediaTrack::FIRE_NO_EVENTS);
     }
   }
 
   // The set of enabled AudioTracks and selected video track might have changed.
   mWatchManager.ManualNotify(&HTMLMediaElement::UpdateReadyStateInternal);
-  mAbstractMainThread->TailDispatcher().AddDirectTask(
+  AbstractThread::DispatchDirectTask(
       NewRunnableMethod("HTMLMediaElement::FirstFrameLoaded", this,
                         &HTMLMediaElement::FirstFrameLoaded));
 }
 
 void HTMLMediaElement::NotifyMediaStreamTrackRemoved(
     const RefPtr<MediaStreamTrack>& aTrack) {
   MOZ_ASSERT(aTrack);
 
--- a/xpcom/threads/AbstractThread.cpp
+++ b/xpcom/threads/AbstractThread.cpp
@@ -45,17 +45,18 @@ class XPCOMThreadWrapper : public Abstra
                   NS_IsMainThread() && aThread->IsOnCurrentThread());
   }
 
   nsresult Dispatch(already_AddRefed<nsIRunnable> aRunnable,
                     DispatchReason aReason = NormalDispatch) override {
     nsCOMPtr<nsIRunnable> r = aRunnable;
     AbstractThread* currentThread;
     if (aReason != TailDispatch && (currentThread = GetCurrent()) &&
-        RequiresTailDispatch(currentThread)) {
+        RequiresTailDispatch(currentThread) &&
+        currentThread->IsTailDispatcherAvailable()) {
       return currentThread->TailDispatcher().AddTask(this, r.forget());
     }
 
     // At a certain point during shutdown, we stop processing events from the
     // main thread event queue (this happens long after all _other_ XPCOM
     // threads have been shut down). However, various bits of subsequent
     // teardown logic (the media shutdown blocker and the final shutdown cycle
     // collection) can trigger state watching and state mirroring notifications
@@ -90,28 +91,37 @@ class XPCOMThreadWrapper : public Abstra
   void FireTailDispatcher() {
     MOZ_DIAGNOSTIC_ASSERT(mTailDispatcher.isSome());
     mTailDispatcher.ref().DrainDirectTasks();
     mTailDispatcher.reset();
   }
 
   TaskDispatcher& TailDispatcher() override {
     MOZ_ASSERT(IsCurrentThreadIn());
+    MOZ_ASSERT(IsTailDispatcherAvailable());
     if (!mTailDispatcher.isSome()) {
       mTailDispatcher.emplace(/* aIsTailDispatcher = */ true);
 
       nsCOMPtr<nsIRunnable> event =
           NewRunnableMethod("XPCOMThreadWrapper::FireTailDispatcher", this,
                             &XPCOMThreadWrapper::FireTailDispatcher);
       nsContentUtils::RunInStableState(event.forget());
     }
 
     return mTailDispatcher.ref();
   }
 
+  bool IsTailDispatcherAvailable() override {
+    // Our tail dispatching implementation relies on nsIThreadObserver
+    // callbacks. If we're not doing event processing, it won't work.
+    bool inEventLoop =
+        static_cast<nsThread*>(mThread.get())->RecursionDepth() > 0;
+    return inEventLoop;
+  }
+
   bool MightHaveTailTasks() override { return mTailDispatcher.isSome(); }
 
   nsIEventTarget* AsEventTarget() override { return mThread; }
 
  private:
   const RefPtr<nsIThreadInternal> mThread;
   Maybe<AutoTaskDispatcher> mTailDispatcher;
 
@@ -232,23 +242,41 @@ void AbstractThread::InitMainThread() {
   }
   // Set the default current main thread so that GetCurrent() never returns
   // nullptr.
   sCurrentThreadTLS.set(sMainThread);
 }
 
 void AbstractThread::DispatchStateChange(
     already_AddRefed<nsIRunnable> aRunnable) {
-  GetCurrent()->TailDispatcher().AddStateChangeTask(this, std::move(aRunnable));
+  AbstractThread* currentThread = GetCurrent();
+  if (currentThread->IsTailDispatcherAvailable()) {
+    currentThread->TailDispatcher().AddStateChangeTask(this,
+                                                       std::move(aRunnable));
+  } else {
+    // If the tail dispatcher isn't available, we just avoid sending state
+    // updates.
+    //
+    // This happens, specifically (1) During async shutdown (via the media
+    // shutdown blocker), and (2) During the final shutdown cycle collection.
+    // Both of these trigger changes to various watched and mirrored state.
+    nsCOMPtr<nsIRunnable> neverDispatched = aRunnable;
+  }
 }
 
 /* static */
 void AbstractThread::DispatchDirectTask(
     already_AddRefed<nsIRunnable> aRunnable) {
-  GetCurrent()->TailDispatcher().AddDirectTask(std::move(aRunnable));
+  AbstractThread* currentThread = GetCurrent();
+  if (currentThread->IsTailDispatcherAvailable()) {
+    currentThread->TailDispatcher().AddDirectTask(std::move(aRunnable));
+  } else {
+    // If the tail dispatcher isn't available, we post as a regular task.
+    currentThread->Dispatch(std::move(aRunnable));
+  }
 }
 
 /* static */
 already_AddRefed<AbstractThread> AbstractThread::CreateXPCOMThreadWrapper(
     nsIThread* aThread, bool aRequireTailDispatch) {
   nsCOMPtr<nsIThreadInternal> internalThread = do_QueryInterface(aThread);
   MOZ_ASSERT(internalThread, "Need an nsThread for AbstractThread");
   RefPtr<XPCOMThreadWrapper> wrapper =
--- a/xpcom/threads/AbstractThread.h
+++ b/xpcom/threads/AbstractThread.h
@@ -77,16 +77,20 @@ class AbstractThread : public nsISerialE
   // May only be called when running within the it is invoked up, and only on
   // threads which support it.
   virtual TaskDispatcher& TailDispatcher() = 0;
 
   // Returns true if we have tail tasks scheduled, or if this isn't known.
   // Returns false if we definitely don't have any tail tasks.
   virtual bool MightHaveTailTasks() { return true; }
 
+  // Returns true if the tail dispatcher is available. In certain edge cases
+  // like shutdown, it might not be.
+  virtual bool IsTailDispatcherAvailable() { return true; }
+
   // Helper functions for methods on the tail TasklDispatcher. These check
   // HasTailTasks to avoid allocating a TailDispatcher if it isn't
   // needed.
   nsresult TailDispatchTasksFor(AbstractThread* aThread);
   bool HasTailTasksFor(AbstractThread* aThread);
 
   // Returns true if this supports the tail dispatcher.
   bool SupportsTailDispatch() const { return mSupportsTailDispatch; }
--- a/xpcom/threads/StateWatching.h
+++ b/xpcom/threads/StateWatching.h
@@ -238,17 +238,17 @@ class WatchManager {
                             "at which point we shouldn't be notified");
       if (mNotificationPending) {
         // We've already got a notification job in the pipe.
         return;
       }
       mNotificationPending = true;
 
       // Queue up our notification jobs to run in a stable state.
-      mOwnerThread->TailDispatcher().AddDirectTask(
+      AbstractThread::DispatchDirectTask(
           NS_NewRunnableFunction("WatchManager::PerCallbackWatcher::Notify",
                                  [self = RefPtr<PerCallbackWatcher>(this),
                                   owner = RefPtr<OwnerType>(mOwner)]() {
                                    if (!self->mDestroyed) {
                                      ((*owner).*(self->mCallbackMethod))();
                                    }
                                    self->mNotificationPending = false;
                                  }));
--- a/xpcom/threads/TaskQueue.cpp
+++ b/xpcom/threads/TaskQueue.cpp
@@ -90,17 +90,18 @@ nsresult TaskQueue::DispatchLocked(nsCOM
                                    uint32_t aFlags, DispatchReason aReason) {
   mQueueMonitor.AssertCurrentThreadOwns();
   if (mIsShutdown) {
     return NS_ERROR_FAILURE;
   }
 
   AbstractThread* currentThread;
   if (aReason != TailDispatch && (currentThread = GetCurrent()) &&
-      RequiresTailDispatch(currentThread)) {
+      RequiresTailDispatch(currentThread) &&
+      currentThread->IsTailDispatcherAvailable()) {
     MOZ_ASSERT(aFlags == NS_DISPATCH_NORMAL,
                "Tail dispatch doesn't support flags");
     return currentThread->TailDispatcher().AddTask(this, aRunnable.forget());
   }
 
   // If the task queue cares about individual flags, retain them in the struct.
   uint32_t retainFlags = mShouldRetainFlags ? aFlags : 0;