Backed out 4 changesets (bug 1768581, bug 1709184) for causing high frequency Xpcshell failures at test_bug1274685_unowned_list.js. CLOSED TREE

Backed out changeset 6bae0f16eae9 (bug 1768581)
Backed out changeset 1f111f5b8baf (bug 1768581)
Backed out changeset a3d89b2ef44a (bug 1709184)
Backed out changeset 98f1dc6f0123 (bug 1709184)

modules/libpref/init/StaticPrefList.yaml

@@ -13599,7 +13599,7 @@
 #ifdef MOZ_CODE_COVERAGE
   value: 0
 #else
-  value: 2
+  value: 3
 #endif
   mirror: always
 

xpcom/base/AppShutdown.cpp

@@ -315,7 +315,6 @@ bool AppShutdown::IsRestarting() {
 
 #ifdef DEBUG
 static bool sNotifyingShutdownObservers = false;
-static bool sAdvancingShutdownPhase = false;
 
 bool AppShutdown::IsNoOrLegalShutdownTopic(const char* aTopic) {
   if (!XRE_IsParentProcess()) {
@@ -333,12 +332,6 @@ void AppShutdown::AdvanceShutdownPhaseInternal(
     ShutdownPhase aPhase, bool doNotify, const char16_t* aNotificationData,
     const nsCOMPtr<nsISupports>& aNotificationSubject) {
   AssertIsOnMainThread();
-#ifdef DEBUG
-  // Prevent us from re-entrance
-  MOZ_ASSERT(!sAdvancingShutdownPhase);
-  sAdvancingShutdownPhase = true;
-  auto exit = MakeScopeExit([] { sAdvancingShutdownPhase = false; });
-#endif
 
   // We ensure that we can move only forward. We cannot
   // MOZ_ASSERT here as there are some tests that fire
@@ -347,43 +340,24 @@ void AppShutdown::AdvanceShutdownPhaseInternal(
   if (sCurrentShutdownPhase >= aPhase) {
     return;
   }
-
-  nsCOMPtr<nsIThread> thread = do_GetCurrentThread();
-  if (sCurrentShutdownPhase >= ShutdownPhase::AppShutdownConfirmed) {
-    // Give runnables dispatched between two calls to AdvanceShutdownPhase
-    // a chance to run before actually advancing the phase. We must do this
-    // only after we passed the point of no return and thus can expect a linear
-    // flow through our shutdown phases. This way the processing is also
-    // covered by the terminator's timer of the previous phase.
-    // Note that this affects only main thread runnables, such that the correct
-    // way of ensuring shutdown processing remains to have an async shutdown
-    // blocker.
-    if (thread) {
-      NS_ProcessPendingEvents(thread);
-    }
-  }
-
-  // From now on any IsInOrBeyond checks will find the new phase set.
   sCurrentShutdownPhase = aPhase;
 
+  // TODO: Bug 1768581
+  // We think it would be more logical to have the following order here:
+  //    AppShutdown::MaybeFastShutdown(aPhase);
+  //    sTerminator->AdvancePhase(aPhase);
+  //    obsService->NotifyObservers(...);
+  //    mozilla::KillClearOnShutdown(aPhase);
+
 #ifndef ANDROID
   if (sTerminator) {
     sTerminator->AdvancePhase(aPhase);
   }
 #endif
 
-  AppShutdown::MaybeFastShutdown(aPhase);
-
-  // This will null out the gathered pointers for this phase synchronously.
-  // Note that we keep the old order here to avoid breakage, so be aware that
-  // the notifications fired below will find these already cleared in case
-  // you expected the opposite.
   mozilla::KillClearOnShutdown(aPhase);
 
-  // Empty our MT event queue to process any side effects thereof.
-  if (thread) {
-    NS_ProcessPendingEvents(thread);
-  }
+  AppShutdown::MaybeFastShutdown(aPhase);
 
   if (doNotify) {
     const char* aTopic = AppShutdown::GetObserverKey(aPhase);
@@ -397,10 +371,6 @@ void AppShutdown::AdvanceShutdownPhaseInternal(
 #endif
         obsService->NotifyObservers(aNotificationSubject, aTopic,
                                     aNotificationData);
-        // Empty our MT event queue again after the notification has finished
-        if (thread) {
-          NS_ProcessPendingEvents(thread);
-        }
       }
     }
   }

xpcom/build/XPCOMInit.cpp

@@ -585,6 +585,7 @@ nsresult ShutdownXPCOM(nsIServiceManager* aServMgr) {
 
     // This must happen after the shutdown of media and widgets, which
     // are triggered by the NS_XPCOM_SHUTDOWN_OBSERVER_ID notification.
+    NS_ProcessPendingEvents(thread);
     gfxPlatform::ShutdownLayersIPC();
 
     mozilla::AppShutdown::AdvanceShutdownPhase(
@@ -594,11 +595,11 @@ nsresult ShutdownXPCOM(nsIServiceManager* aServMgr) {
     // dispatches to non main-thread threads.
     ThreadEventTarget::XPCOMShutdownThreadsNotificationFinished();
 #endif
+    NS_ProcessPendingEvents(thread);
 
     // Shutdown the timer thread and all timers that might still be alive
     nsTimerImpl::Shutdown();
 
-    // Have an extra round of processing after the timers went away.
     NS_ProcessPendingEvents(thread);
 
     // Shutdown all remaining threads.  This method does not return until
@@ -616,11 +617,13 @@ nsresult ShutdownXPCOM(nsIServiceManager* aServMgr) {
     // XPCOMShutdownFinal is the default phase for ClearOnShutdown.
     // This AdvanceShutdownPhase will thus free most ClearOnShutdown()'ed
     // smart pointers. Some destructors may fire extra main thread runnables
-    // that will be processed inside AdvanceShutdownPhase.
+    // that will be processed below.
     AppShutdown::AdvanceShutdownPhase(ShutdownPhase::XPCOMShutdownFinal);
 
-    // Shutdown the main thread, processing our very last round of events, and
-    // then mark that we've finished main thread event processing.
+    NS_ProcessPendingEvents(thread);
+
+    // Shutdown the main thread, processing our last round of events, and then
+    // mark that we've finished main thread event processing.
     nsThreadManager::get().ShutdownMainThread();
     gXPCOMMainThreadEventsAreDoomed = true;
 
@@ -629,6 +632,8 @@ nsresult ShutdownXPCOM(nsIServiceManager* aServMgr) {
     mozilla::dom::JSExecutionManager::Shutdown();
   }
 
+  AbstractThread::ShutdownMainThread();
+
   // XPCOM is officially in shutdown mode NOW
   // Set this only after the observers have been notified as this
   // will cause servicemanager to become inaccessible.
@@ -643,10 +648,6 @@ nsresult ShutdownXPCOM(nsIServiceManager* aServMgr) {
     nsComponentManagerImpl::gComponentManager->FreeServices();
   }
 
-  // Remove the remaining main thread representations
-  nsThreadManager::get().ReleaseMainThread();
-  AbstractThread::ShutdownMainThread();
-
   // Release the directory service
   nsDirectoryService::gService = nullptr;
 

xpcom/threads/nsThreadManager.cpp

@@ -421,12 +421,6 @@ void nsThreadManager::ShutdownMainThread() {
   mMainThread->SetObserver(nullptr);
 
   mBackgroundEventTarget = nullptr;
-}
-
-void nsThreadManager::ReleaseMainThread() {
-  MOZ_ASSERT(!mInitialized, "Must have called BeginShutdown");
-  MOZ_ASSERT(!mBackgroundEventTarget, "Must have called ShutdownMainThread");
-  MOZ_ASSERT(mMainThread);
 
   // Release main thread object.
   mMainThread = nullptr;

xpcom/threads/nsThreadManager.h

@@ -37,14 +37,9 @@ class nsThreadManager : public nsIThreadManager {
   void ShutdownNonMainThreads();
 
   // Finish shutting down all threads. This function must be called after
-  // ShutdownNonMainThreads and will delete the BackgroundEventTarget and
-  // take the main thread event target out of commission, but without
-  // releasing the underlying nsThread object.
+  // ShutdownNonMainThreads and will take the main thread out of commission.
   void ShutdownMainThread();
 
-  // Release the underlying main thread nsThread object.
-  void ReleaseMainThread();
-
   // Called by nsThread to inform the ThreadManager it exists.  This method
   // must be called when the given thread is the current thread.
   void RegisterCurrentThread(nsThread& aThread);