Bug 1584568 - add an API to construct background task queues; r=KrisWright

For some clients, just dispatching tasks to some anonymous background
thread is fine.  But for other clients, they need to guarantee that
dispatched events are executed in dispatch order, or they would like to
have some guarantee that functions executing in the background are
executing on a particular event target, or both.  For such uses cases,
we need something a little more sophisticated than simply handing out
the `BackgroundEventTarget` `nsThreadManager` is using.

Fortunately, we have an abstraction that provides these sorts of
guarantees already in `mozilla::TaskQueue`.  Since `mozilla::TaskQueue`
requires a bit of special care during shutdown, we're not going to hand
out new `TaskQueue` objects directly, but will instead hand out
`nsISerialEventTarget` wrappers of the newly-created `TaskQueues`.
`nsThreadManager` can then take care of shutting down all of the
`TaskQueue` objects itself, rather than requiring clients to handle
shutdown themselves.

Differential Revision: https://phabricator.services.mozilla.com/D47454

--HG--
extra : moz-landing-system : lando

xpcom/threads/nsThreadManager.cpp

@@ -16,9 +16,11 @@
 #include "mozilla/AbstractThread.h"
 #include "mozilla/ClearOnShutdown.h"
 #include "mozilla/EventQueue.h"
+#include "mozilla/Mutex.h"
 #include "mozilla/Preferences.h"
 #include "mozilla/SystemGroup.h"
 #include "mozilla/StaticPtr.h"
+#include "mozilla/TaskQueue.h"
 #include "mozilla/ThreadEventQueue.h"
 #include "mozilla/ThreadLocal.h"
 #include "PrioritizedEventQueue.h"
@@ -41,21 +43,32 @@ class BackgroundEventTarget final : public nsIEventTarget {
   NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSIEVENTTARGET_FULL
 
-  BackgroundEventTarget() = default;
+  BackgroundEventTarget();
 
   nsresult Init();
 
-  nsresult Shutdown();
+  already_AddRefed<nsISerialEventTarget>
+    CreateBackgroundTaskQueue(const char* aName);
+
+  void BeginShutdown(nsTArray<RefPtr<ShutdownPromise>>&);
+  void FinishShutdown();
 
  private:
   ~BackgroundEventTarget() = default;
 
   nsCOMPtr<nsIThreadPool> mPool;
   nsCOMPtr<nsIThreadPool> mIOPool;
+
+  Mutex mMutex;
+  nsTArray<RefPtr<TaskQueue>> mTaskQueues;
 };
 
 NS_IMPL_ISUPPORTS(BackgroundEventTarget, nsIEventTarget)
 
+BackgroundEventTarget::BackgroundEventTarget()
+    : mMutex("BackgroundEventTarget::mMutex")
+{}
+
 nsresult BackgroundEventTarget::Init() {
   nsCOMPtr<nsIThreadPool> pool(new nsThreadPool());
   NS_ENSURE_TRUE(pool, NS_ERROR_FAILURE);
@@ -118,11 +131,29 @@ BackgroundEventTarget::IsOnCurrentThread(bool* aValue) {
 NS_IMETHODIMP
 BackgroundEventTarget::Dispatch(already_AddRefed<nsIRunnable> aRunnable,
                                 uint32_t aFlags) {
+  // We need to be careful here, because if an event is getting dispatched here
+  // from within TaskQueue::Runner::Run, it will be dispatched with
+  // NS_DISPATCH_AT_END, but we might not be running the event on the same
+  // pool, depending on which pool we were on and the dispatch flags.  If we
+  // dispatch an event with NS_DISPATCH_AT_END to the wrong pool, the pool
+  // may not process the event in a timely fashion, which can lead to deadlock.
   uint32_t flags = aFlags & ~NS_DISPATCH_EVENT_MAY_BLOCK;
-  if (aFlags & NS_DISPATCH_EVENT_MAY_BLOCK) {
-    return mIOPool->Dispatch(std::move(aRunnable), flags);
+  bool mayBlock = bool(aFlags & NS_DISPATCH_EVENT_MAY_BLOCK);
+  nsCOMPtr<nsIThreadPool>& pool = mayBlock ? mIOPool : mPool;
+
+  // If we're already running on the pool we want to dispatch to, we can
+  // unconditionally add NS_DISPATCH_AT_END to indicate that we shouldn't spin
+  // up a new thread.
+  //
+  // Otherwise, we should remove NS_DISPATCH_AT_END so we don't run into issues
+  // like those in the above comment.
+  if (pool->IsOnCurrentThread()) {
+    flags |= NS_DISPATCH_AT_END;
+  } else {
+    flags &= ~NS_DISPATCH_AT_END;
   }
-  return mPool->Dispatch(std::move(aRunnable), flags);
+
+  return pool->Dispatch(std::move(aRunnable), flags);
 }
 
 NS_IMETHODIMP
@@ -139,10 +170,28 @@ BackgroundEventTarget::DelayedDispatch(already_AddRefed<nsIRunnable> aRunnable,
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
-nsresult BackgroundEventTarget::Shutdown() {
+void BackgroundEventTarget::BeginShutdown(nsTArray<RefPtr<ShutdownPromise>>& promises) {
+  for (auto& queue : mTaskQueues) {
+    promises.AppendElement(queue->BeginShutdown());
+  }
+}
+
+void BackgroundEventTarget::FinishShutdown() {
   mPool->Shutdown();
   mIOPool->Shutdown();
-  return NS_OK;
+}
+
+already_AddRefed<nsISerialEventTarget> BackgroundEventTarget::CreateBackgroundTaskQueue(const char* aName) {
+  MutexAutoLock lock(mMutex);
+
+  RefPtr<TaskQueue> queue = new TaskQueue(do_AddRef(this), aName,
+                                          /*aSupportsTailDispatch=*/ false,
+                                          /*aRetainFlags=*/ true);
+  nsCOMPtr<nsISerialEventTarget> target(queue->WrapAsEventTarget());
+
+  mTaskQueues.AppendElement(queue.forget());
+
+  return target.forget();
 }
 
 extern "C" {
@@ -294,6 +343,8 @@ void nsThreadManager::InitializeShutdownObserver() {
 nsThreadManager::nsThreadManager()
     : mCurThreadIndex(0), mMainPRThread(nullptr), mInitialized(false) {}
 
+nsThreadManager::~nsThreadManager() = default;
+
 nsresult nsThreadManager::Init() {
   // Child processes need to initialize the thread manager before they
   // initialize XPCOM in order to set up the crash reporter. This leads to
@@ -363,7 +414,33 @@ void nsThreadManager::Shutdown() {
   // Empty the main thread event queue before we begin shutting down threads.
   NS_ProcessPendingEvents(mMainThread);
 
-  static_cast<BackgroundEventTarget*>(mBackgroundEventTarget.get())->Shutdown();
+  typedef typename ShutdownPromise::AllPromiseType AllPromise;
+  typename AllPromise::ResolveOrRejectValue val;
+  using ResolveValueT = typename AllPromise::ResolveValueType;
+  using RejectValueT = typename AllPromise::RejectValueType;
+
+  nsTArray<RefPtr<ShutdownPromise>> promises;
+  mBackgroundEventTarget->BeginShutdown(promises);
+
+  RefPtr<AllPromise> complete = ShutdownPromise::All(mMainThread, promises);
+
+  bool taskQueuesShutdown = false;
+
+  complete->Then(mMainThread, __func__,
+                 [&](const ResolveValueT& aResolveValue) {
+                   mBackgroundEventTarget->FinishShutdown();
+                   taskQueuesShutdown = true;
+                 },
+                 [&](RejectValueT aRejectValue) {
+                   mBackgroundEventTarget->FinishShutdown();
+                   taskQueuesShutdown = true;
+                 });
+
+  // Wait for task queues to shutdown, so we don't shut down the underlying threads
+  // of the background event target in the block below, thereby preventing the task
+  // queues from emptying, preventing the shutdown promises from resolving, and
+  // prevent anything checking `taskQueuesShutdown` from working.
+  ::SpinEventLoopUntil([&]() { return taskQueuesShutdown; }, mMainThread);
 
   {
     // We gather the threads from the hashtable into a list, so that we avoid
@@ -475,6 +552,14 @@ nsresult nsThreadManager::DispatchToBackgroundThread(nsIRunnable* aEvent,
   return backgroundTarget->Dispatch(aEvent, aDispatchFlags);
 }
 
+already_AddRefed<nsISerialEventTarget> nsThreadManager::CreateBackgroundTaskQueue(const char* aName) {
+  if (!mInitialized) {
+    return nullptr;
+  }
+
+  return mBackgroundEventTarget->CreateBackgroundTaskQueue(aName);
+}
+
 nsThread* nsThreadManager::GetCurrentThread() {
   // read thread local storage
   void* data = PR_GetThreadPrivate(mCurThreadIndex);

xpcom/threads/nsThreadManager.h

@@ -13,6 +13,8 @@
 
 class nsIRunnable;
 
+class BackgroundEventTarget;
+
 class nsThreadManager : public nsIThreadManager {
  public:
   NS_DECL_ISUPPORTS
@@ -56,13 +58,13 @@ class nsThreadManager : public nsIThreadManager {
   nsresult DispatchToBackgroundThread(nsIRunnable* aEvent,
                                       uint32_t aDispatchFlags);
 
+  already_AddRefed<nsISerialEventTarget> CreateBackgroundTaskQueue(const char* aName);
+
   // Returns the maximal number of threads that have been in existence
   // simultaneously during the execution of the thread manager.
   uint32_t GetHighestNumberOfThreads();
 
-  // This needs to be public in order to support static instantiation of this
-  // class with older compilers (e.g., egcs-2.91.66).
-  ~nsThreadManager() {}
+  ~nsThreadManager();
 
   void EnableMainThreadEventPrioritization();
   void FlushInputEventPrioritization();
@@ -87,7 +89,7 @@ class nsThreadManager : public nsIThreadManager {
       mInitialized;
 
   // Shared event target used for background runnables.
-  nsCOMPtr<nsIEventTarget> mBackgroundEventTarget;
+  RefPtr<BackgroundEventTarget> mBackgroundEventTarget;
 };
 
 #define NS_THREADMANAGER_CID                         \

xpcom/threads/nsThreadUtils.cpp

@@ -531,6 +531,18 @@ nsresult NS_DispatchBackgroundTask(nsIRunnable* aEvent,
                                                            aDispatchFlags);
 }
 
+nsresult NS_CreateBackgroundTaskQueue(const char* aName,
+                                      nsISerialEventTarget** aTarget) {
+  nsCOMPtr<nsISerialEventTarget> target =
+    nsThreadManager::get().CreateBackgroundTaskQueue(aName);
+  if (!target) {
+    return NS_ERROR_FAILURE;
+  }
+
+  target.forget(aTarget);
+  return NS_OK;
+}
+
 // nsAutoLowPriorityIO
 nsAutoLowPriorityIO::nsAutoLowPriorityIO() {
 #if defined(XP_WIN)

xpcom/threads/nsThreadUtils.h

@@ -1700,6 +1700,12 @@ extern mozilla::TimeStamp NS_GetTimerDeadlineHintOnCurrentThread(
  * background thread's lifetime.  Not having to manage your own thread also
  * means less resource usage, as the underlying implementation here can manage
  * spinning up and shutting down threads appropriately.
+ *
+ * NOTE: there is no guarantee that events dispatched via these APIs are run
+ * serially, in dispatch order; several dispatched events may run in parallel.
+ * If you depend on serial execution of dispatched events, you should use
+ * NS_CreateBackgroundTaskQueue instead, and dispatch events to the returned
+ * event target.
  */
 extern nsresult NS_DispatchBackgroundTask(
     already_AddRefed<nsIRunnable> aEvent,
@@ -1707,6 +1713,15 @@ extern nsresult NS_DispatchBackgroundTask(
 extern nsresult NS_DispatchBackgroundTask(
     nsIRunnable* aEvent, uint32_t aDispatchFlags = NS_DISPATCH_NORMAL);
 
+/**
+ * Obtain a new serial event target that dispatches runnables to a background
+ * thread.  In many cases, this is a straight replacement for creating your
+ * own, private thread, and is generally preferred to creating your own,
+ * private thread.
+ */
+extern nsresult NS_CreateBackgroundTaskQueue(const char* aName,
+                                             nsISerialEventTarget** aTarget);
+
 namespace mozilla {
 
 // These functions return event targets that can be used to dispatch to the