Bug 1200922: Add the ability to shut down a thread asynchronously. r=froydnj

xpcom/tests/gtest/TestThreads.cpp

@@ -11,6 +11,7 @@
 #include "nsCOMPtr.h"
 #include "nsIServiceManager.h"
 #include "nsXPCOM.h"
+#include "mozilla/Monitor.h"
 #include "gtest/gtest.h"
 
 class nsRunner final : public nsIRunnable {
@@ -124,6 +125,116 @@ TEST(Threads, Stress)
     }
 }
 
+mozilla::Monitor* gAsyncShutdownReadyMonitor;
+mozilla::Monitor* gBeginAsyncShutdownMonitor;
+
+class AsyncShutdownPreparer : public nsIRunnable {
+public:
+    NS_DECL_THREADSAFE_ISUPPORTS
+
+    NS_IMETHOD Run() override {
+        EXPECT_FALSE(mWasRun);
+        mWasRun = true;
+
+        mozilla::MonitorAutoLock lock(*gAsyncShutdownReadyMonitor);
+        lock.Notify();
+
+        return NS_OK;
+    }
+
+    explicit AsyncShutdownPreparer() : mWasRun(false) {}
+
+private:
+    virtual ~AsyncShutdownPreparer() {
+        EXPECT_TRUE(mWasRun);
+    }
+
+protected:
+    bool mWasRun;
+};
+
+NS_IMPL_ISUPPORTS(AsyncShutdownPreparer, nsIRunnable)
+
+class AsyncShutdownWaiter : public nsIRunnable {
+public:
+    NS_DECL_THREADSAFE_ISUPPORTS
+
+    NS_IMETHOD Run() override {
+        EXPECT_FALSE(mWasRun);
+        mWasRun = true;
+
+        nsCOMPtr<nsIThread> t;
+        nsresult rv;
+
+        {
+          mozilla::MonitorAutoLock lock(*gBeginAsyncShutdownMonitor);
+
+          rv = NS_NewThread(getter_AddRefs(t), new AsyncShutdownPreparer());
+          EXPECT_TRUE(NS_SUCCEEDED(rv));
+
+          lock.Wait();
+        }
+
+        rv = t->AsyncShutdown();
+        EXPECT_TRUE(NS_SUCCEEDED(rv));
+
+        return NS_OK;
+    }
+
+    explicit AsyncShutdownWaiter() : mWasRun(false) {}
+
+private:
+    virtual ~AsyncShutdownWaiter() {
+        EXPECT_TRUE(mWasRun);
+    }
+
+protected:
+    bool mWasRun;
+};
+
+NS_IMPL_ISUPPORTS(AsyncShutdownWaiter, nsIRunnable)
+
+class SameThreadSentinel : public nsIRunnable {
+public:
+    NS_DECL_ISUPPORTS
+
+    NS_IMETHOD Run() override {
+        mozilla::MonitorAutoLock lock(*gBeginAsyncShutdownMonitor);
+        lock.Notify();
+        return NS_OK;
+    }
+
+private:
+    virtual ~SameThreadSentinel() {}
+};
+
+NS_IMPL_ISUPPORTS(SameThreadSentinel, nsIRunnable)
+
+TEST(Threads, AsyncShutdown)
+{
+  gAsyncShutdownReadyMonitor = new mozilla::Monitor("gAsyncShutdownReady");
+  gBeginAsyncShutdownMonitor = new mozilla::Monitor("gBeginAsyncShutdown");
+
+  nsCOMPtr<nsIThread> t;
+  nsresult rv;
+
+  {
+    mozilla::MonitorAutoLock lock(*gAsyncShutdownReadyMonitor);
+
+    rv = NS_NewThread(getter_AddRefs(t), new AsyncShutdownWaiter());
+    EXPECT_TRUE(NS_SUCCEEDED(rv));
+
+    lock.Wait();
+  }
+
+  NS_DispatchToCurrentThread(new SameThreadSentinel());
+  rv = t->Shutdown();
+  EXPECT_TRUE(NS_SUCCEEDED(rv));
+
+  delete gAsyncShutdownReadyMonitor;
+  delete gBeginAsyncShutdownMonitor;
+}
+
 static void threadProc(void *arg)
 {
     // printf("   running thread %d\n", (int) arg);

xpcom/threads/LazyIdleThread.cpp

@@ -456,6 +456,13 @@ LazyIdleThread::GetPRThread(PRThread** aPRThread)
   return NS_ERROR_NOT_AVAILABLE;
 }
 
+NS_IMETHODIMP
+LazyIdleThread::AsyncShutdown()
+{
+  ASSERT_OWNING_THREAD();
+  return NS_ERROR_NOT_IMPLEMENTED;
+}
+
 NS_IMETHODIMP
 LazyIdleThread::Shutdown()
 {

xpcom/threads/nsIThread.idl

@@ -17,7 +17,7 @@
  *
  * See nsIThreadManager for the API used to create and locate threads.
  */
-[scriptable, uuid(9c889946-a73a-4af3-ae9a-ea64f7d4e3ca)]
+[scriptable, uuid(594feb13-6164-4054-b5a1-ad62e10ea15d)]
 interface nsIThread : nsIEventTarget
 {
   /**
@@ -82,4 +82,26 @@ interface nsIThread : nsIEventTarget
    *   not the current thread.
    */
   boolean processNextEvent(in boolean mayWait);
+
+  /**
+   * Shutdown the thread asynchronously.  This method immediately prevents
+   * further dispatch of events to the thread, and it causes any pending events
+   * to run to completion before this thread joins with the current thread.
+   *
+   * UNLIKE shutdown() this does not process events on the current thread.
+   * Instead it merely ensures that the current thread continues running until
+   * this thread has shut down.
+   *
+   * This method MAY NOT be executed from the thread itself.  Instead, it is
+   * meant to be executed from another thread (usually the thread that created
+   * this thread or the main application thread).  When this function returns,
+   * the thread will continue running until it exhausts its event queue.
+   *
+   * @throws NS_ERROR_UNEXPECTED
+   *   Indicates that this method was erroneously called when this thread was
+   *   the current thread, that this thread was not created with a call to
+   *   nsIThreadManager::NewThread, or if this method was called more than once
+   *   on the thread object.
+   */
+  void asyncShutdown();
 };

xpcom/threads/nsThread.cpp

@@ -21,6 +21,7 @@
 #include "nsIClassInfoImpl.h"
 #include "nsAutoPtr.h"
 #include "nsCOMPtr.h"
+#include "nsQueryObject.h"
 #include "pratom.h"
 #include "mozilla/CycleCollectedJSRuntime.h"
 #include "mozilla/Logging.h"
@@ -238,28 +239,43 @@ private:
 
 struct nsThreadShutdownContext
 {
+  // NB: This will be the last reference.
+  nsRefPtr<nsThread> terminatingThread;
   nsThread* joiningThread;
-  bool      shutdownAck;
+  bool      awaitingShutdownAck;
 };
 
 // This event is responsible for notifying nsThread::Shutdown that it is time
-// to call PR_JoinThread.
-class nsThreadShutdownAckEvent : public nsRunnable
+// to call PR_JoinThread. It implements nsICancelableRunnable so that it can
+// run on a DOM Worker thread (where all events must implement
+// nsICancelableRunnable.)
+class nsThreadShutdownAckEvent : public nsRunnable,
+                                 public nsICancelableRunnable
 {
 public:
   explicit nsThreadShutdownAckEvent(nsThreadShutdownContext* aCtx)
     : mShutdownContext(aCtx)
   {
   }
-  NS_IMETHOD Run()
+  NS_DECL_ISUPPORTS_INHERITED
+  NS_IMETHOD Run() override
   {
-    mShutdownContext->shutdownAck = true;
+    mShutdownContext->terminatingThread->ShutdownComplete(mShutdownContext);
     return NS_OK;
   }
+  NS_IMETHOD Cancel() override
+  {
+    return Run();
+  }
 private:
+  virtual ~nsThreadShutdownAckEvent() { }
+
   nsThreadShutdownContext* mShutdownContext;
 };
 
+NS_IMPL_ISUPPORTS_INHERITED(nsThreadShutdownAckEvent, nsRunnable,
+                            nsICancelableRunnable)
+
 // This event is responsible for setting mShutdownContext
 class nsThreadShutdownEvent : public nsRunnable
 {
@@ -369,8 +385,15 @@ nsThread::ThreadFunc(void* aArg)
     // 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.
+    // 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);
+      }
+
       {
         MutexAutoLock lock(self->mLock);
         if (!self->mEvents->HasPendingEvent()) {
@@ -394,7 +417,8 @@ nsThread::ThreadFunc(void* aArg)
   nsThreadManager::get()->UnregisterCurrentThread(self);
 
   // Dispatch shutdown ACK
-  event = new nsThreadShutdownAckEvent(self->mShutdownContext);
+  MOZ_ASSERT(self->mShutdownContext->terminatingThread == self);
+  event = do_QueryObject(new nsThreadShutdownAckEvent(self->mShutdownContext));
   self->mShutdownContext->joiningThread->Dispatch(event, NS_DISPATCH_NORMAL);
 
   // Release any observer of the thread here.
@@ -631,9 +655,9 @@ nsThread::GetPRThread(PRThread** aResult)
 }
 
 NS_IMETHODIMP
-nsThread::Shutdown()
+nsThread::AsyncShutdown()
 {
-  LOG(("THRD(%p) shutdown\n", this));
+  LOG(("THRD(%p) async 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
@@ -642,37 +666,61 @@ nsThread::Shutdown()
     return NS_OK;
   }
 
+  return !!ShutdownInternal(/* aSync = */ false) ? NS_OK : NS_ERROR_UNEXPECTED;
+}
+
+nsThreadShutdownContext*
+nsThread::ShutdownInternal(bool aSync)
+{
+  MOZ_ASSERT(mThread);
+
   if (NS_WARN_IF(mThread == PR_GetCurrentThread())) {
-    return NS_ERROR_UNEXPECTED;
+    return nullptr;
   }
 
   // Prevent multiple calls to this method
   {
     MutexAutoLock lock(mLock);
     if (!mShutdownRequired) {
-      return NS_ERROR_UNEXPECTED;
+      return nullptr;
     }
     mShutdownRequired = false;
   }
 
-  nsThreadShutdownContext context;
-  context.joiningThread = nsThreadManager::get()->GetCurrentThread();
-  context.shutdownAck = false;
+  nsThread* currentThread = nsThreadManager::get()->GetCurrentThread();
+  MOZ_ASSERT(currentThread);
+
+  nsAutoPtr<nsThreadShutdownContext>& context =
+    *currentThread->mRequestedShutdownContexts.AppendElement();
+  context = new nsThreadShutdownContext();
+
+  context->terminatingThread = this;
+  context->joiningThread = currentThread;
+  context->awaitingShutdownAck = aSync;
 
   // Set mShutdownContext and wake up the thread in case it is waiting for
   // events to process.
-  nsCOMPtr<nsIRunnable> event = new nsThreadShutdownEvent(this, &context);
+  nsCOMPtr<nsIRunnable> event = new nsThreadShutdownEvent(this, context);
   // XXXroc What if posting the event fails due to OOM?
   PutEvent(event.forget(), nullptr);
 
   // We could still end up with other events being added after the shutdown
   // task, but that's okay because we process pending events in ThreadFunc
   // after setting mShutdownContext just before exiting.
+  return context;
+}
 
-  // Process events on the current thread until we receive a shutdown ACK.
-  // Allows waiting; ensure no locks are held that would deadlock us!
-  while (!context.shutdownAck) {
-    NS_ProcessNextEvent(context.joiningThread, true);
+void
+nsThread::ShutdownComplete(nsThreadShutdownContext* aContext)
+{
+  MOZ_ASSERT(mThread);
+  MOZ_ASSERT(aContext->terminatingThread == this);
+
+  if (aContext->awaitingShutdownAck) {
+    // We're in a synchronous shutdown, so tell whatever is up the stack that
+    // we're done and unwind the stack so it can call us again.
+    aContext->awaitingShutdownAck = false;
+    return;
   }
 
   // Now, it should be safe to join without fear of dead-locking.
@@ -692,6 +740,34 @@ nsThread::Shutdown()
   }
 #endif
 
+  // Delete aContext.
+  MOZ_ALWAYS_TRUE(
+    aContext->joiningThread->mRequestedShutdownContexts.RemoveElement(aContext));
+}
+
+NS_IMETHODIMP
+nsThread::Shutdown()
+{
+  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.
+  if (!mThread) {
+    return NS_OK;
+  }
+
+  nsThreadShutdownContext* context = ShutdownInternal(/* aSync = */ true);
+  NS_ENSURE_TRUE(context, NS_ERROR_UNEXPECTED);
+
+  // Process events on the current thread until we receive a shutdown ACK.
+  // Allows waiting; ensure no locks are held that would deadlock us!
+  while (context->awaitingShutdownAck) {
+    NS_ProcessNextEvent(context->joiningThread, true);
+  }
+
+  ShutdownComplete(context);
+
   return NS_OK;
 }
 

xpcom/threads/nsThread.h

@@ -77,6 +77,8 @@ public:
   uint32_t
   RecursionDepth() const;
 
+  void ShutdownComplete(struct nsThreadShutdownContext* aContext);
+
 protected:
   class nsChainedEventQueue;
 
@@ -113,6 +115,8 @@ protected:
   nsresult DispatchInternal(already_AddRefed<nsIRunnable>&& aEvent, uint32_t aFlags,
                             nsNestedEventTarget* aTarget);
 
+  struct nsThreadShutdownContext* ShutdownInternal(bool aSync);
+
   // Wrapper for nsEventQueue that supports chaining.
   class nsChainedEventQueue
   {
@@ -193,7 +197,10 @@ protected:
   uint32_t  mNestedEventLoopDepth;
   uint32_t  mStackSize;
 
+  // The shutdown context for ourselves.
   struct nsThreadShutdownContext* mShutdownContext;
+  // The shutdown contexts for any other threads we've asked to shut down.
+  nsTArray<nsAutoPtr<struct nsThreadShutdownContext>> mRequestedShutdownContexts;
 
   bool mShutdownRequired;
   // Set to true when events posted to this thread will never run.

xpcom/threads/nsThreadPool.cpp

@@ -126,15 +126,9 @@ nsThreadPool::PutEvent(already_AddRefed<nsIRunnable>&& aEvent)
   }
   LOG(("THRD-P(%p) put [%p kill=%d]\n", this, thread.get(), killThread));
   if (killThread) {
-    // Pending events are processed on the current thread during
-    // nsIThread::Shutdown() execution, so if nsThreadPool::Dispatch() is called
-    // under caller's lock then deadlock could occur. This happens e.g. in case
-    // of nsStreamCopier. To prevent this situation, dispatch a shutdown event
-    // to the current thread instead of calling nsIThread::Shutdown() directly.
-
-    nsCOMPtr<nsIRunnable> r = NS_NewRunnableMethod(thread,
-                                                   &nsIThread::Shutdown);
-    NS_DispatchToCurrentThread(r);
+    // We never dispatched any events to the thread, so we can shut it down
+    // asynchronously without worrying about anything.
+    MOZ_ALWAYS_TRUE(NS_SUCCEEDED(thread->AsyncShutdown()));
   } else {
     thread->Dispatch(this, NS_DISPATCH_NORMAL);
   }