Bug 1204784: Do not shut the main thread down before all outstanding asynchronous thread shutdowns complete. r=froydnj
authorKyle Huey <khuey@kylehuey.com>
Mon, 18 Jan 2016 09:34:38 -0800
changeset 280394 c395b4777d7dd95a224f3fef2a3d9d6c47b2b85b
parent 280393 6ea1cb521fc5052c95e79c8fc41368b3c37b4711
child 280395 889b91afef87d8c365ff1800a8a260f52d51113e
push id70427
push userkhuey@mozilla.com
push dateMon, 18 Jan 2016 17:35:40 +0000
treeherdermozilla-inbound@c395b4777d7d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
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 1204784: Do not shut the main thread down before all outstanding asynchronous thread shutdowns complete. r=froydnj
--- a/xpcom/threads/nsThread.cpp
+++ b/xpcom/threads/nsThread.cpp
@@ -398,28 +398,28 @@ nsThread::ThreadFunc(void* aArg)
       new MessageLoop(MessageLoop::TYPE_MOZILLA_NONMAINTHREAD));
     // Now, process incoming events...
 #endif // defined(MOZILLA_XPCOMRT_API)
+    // NB: The main thread does not shut down here!  It shuts down via
+    // nsThreadManager::Shutdown.
     // Do NS_ProcessPendingEvents but with special handling to set
     // mEventsAreDoomed atomically with the removal of the last event. The key
     // invariant here is that we will never permit PutEvent to succeed if the
     // event would be left in the queue after our final call to
     // NS_ProcessPendingEvents. We also have to keep processing events as long
     // as we have outstanding mRequestedShutdownContexts.
     while (true) {
       // Check and see if we're waiting on any threads.
-      while (self->mRequestedShutdownContexts.Length()) {
-        // We can't stop accepting events just yet.  Block and check again.
-        NS_ProcessNextEvent(self, true);
-      }
+      self->WaitForAllAsynchronousShutdowns();
         MutexAutoLock lock(self->mLock);
         if (!self->mEvents->HasPendingEvent(lock)) {
           // No events in the queue, so we will stop now. Don't let any more
           // events be added, since they won't be processed. It is critical
           // that no PutEvent can occur between testing that the event queue is
           // empty and setting mEventsAreDoomed!
@@ -773,16 +773,24 @@ nsThread::ShutdownComplete(nsThreadShutd
   // Delete aContext.
+  while (mRequestedShutdownContexts.Length()) {
+    NS_ProcessNextEvent(this, true);
+  }
   LOG(("THRD(%p) sync shutdown\n", this));
   // XXX If we make this warn, then we hit that warning at xpcom shutdown while
   //     shutting down a thread in a thread pool.  That happens b/c the thread
   //     in the thread pool is already shutdown by the thread manager.
--- a/xpcom/threads/nsThread.h
+++ b/xpcom/threads/nsThread.h
@@ -71,16 +71,18 @@ public:
   SetScriptObserver(mozilla::CycleCollectedJSRuntime* aScriptObserver);
   RecursionDepth() const;
   void ShutdownComplete(struct nsThreadShutdownContext* aContext);
+  void WaitForAllAsynchronousShutdowns();
   class nsChainedEventQueue;
   class nsNestedEventTarget;
   friend class nsNestedEventTarget;
   friend class nsThreadShutdownEvent;
--- a/xpcom/threads/nsThreadManager.cpp
+++ b/xpcom/threads/nsThreadManager.cpp
@@ -150,16 +150,22 @@ nsThreadManager::Shutdown()
   // Shutdown all threads that require it (join with threads that we created).
   for (uint32_t i = 0; i < threads.Length(); ++i) {
     nsThread* thread = threads[i];
     if (thread->ShutdownRequired()) {
+  // NB: It's possible that there are events in the queue that want to *start*
+  // an asynchronous shutdown. But we have already shutdown the threads above,
+  // so there's no need to worry about them. We only have to wait for all
+  // in-flight asynchronous thread shutdowns to complete.
+  mMainThread->WaitForAllAsynchronousShutdowns();
   // In case there are any more events somehow...
   // There are no more background threads at this point.
   // Clear the table of threads.
     OffTheBooksMutexAutoLock lock(mLock);