Bug 1770451 - Release assert if a thread shutdown is unexpectedly going to cause a hang on the joining thread. r=xpcom-reviewers,nika

There is a code path that can make us not dispatch the `nsThreadShutdownAckEvent` at the end of `nsThread::ThreadFunc`. The only known and legitimate way to arrive here should be via StopWaitingAndLeakThread. We want to ensure that if we take this code path someone else already marked the shutdown context as complete.

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

xpcom/threads/nsThread.cpp

@@ -421,8 +421,9 @@ void nsThread::ThreadFunc(void* aArg) {
   // which case we won't notify our caller, and leak.
   RefPtr<nsThread> joiningThread;
   {
-    auto lock = context->mJoiningThread.Lock();
-    joiningThread = lock->forget();
+    MutexAutoLock lock(context->mJoiningThreadMutex);
+    joiningThread = context->mJoiningThread.forget();
+    MOZ_RELEASE_ASSERT(joiningThread || context->mThreadLeaked);
   }
   if (joiningThread) {
     // Dispatch shutdown ACK
@@ -834,6 +835,17 @@ void nsThread::ShutdownComplete(NotNull<nsThreadShutdownContext*> aContext) {
   MOZ_ASSERT(mEventTarget);
   MOZ_ASSERT(aContext->mTerminatingThread == this);
 
+#ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED
+  {
+    MutexAutoLock lock(aContext->mJoiningThreadMutex);
+
+    // StopWaitingAndLeakThread is explicitely meant to not cause a
+    // nsThreadShutdownAckEvent on the joining thread, which is the only
+    // caller of ShutdownComplete.
+    MOZ_DIAGNOSTIC_ASSERT(!aContext->mThreadLeaked);
+  }
+#endif
+
   MaybeRemoveFromThreadList();
 
   // Now, it should be safe to join without fear of dead-locking.
@@ -1445,16 +1457,19 @@ nsThreadShutdownContext::StopWaitingAndLeakThread() {
   // thread won't try to dispatch nsThreadShutdownAckEvent to us anymore.
   RefPtr<nsThread> joiningThread;
   {
-    auto lock = mJoiningThread.Lock();
-    joiningThread = lock->forget();
-  }
-  if (!joiningThread) {
-    // Shutdown is already being resolved, so there's nothing for us to do.
-    return NS_ERROR_NOT_AVAILABLE;
+    MutexAutoLock lock(mJoiningThreadMutex);
+    if (!mJoiningThread) {
+      // Shutdown is already being resolved, so there's nothing for us to do.
+      return NS_ERROR_NOT_AVAILABLE;
+    }
+    joiningThread = mJoiningThread.forget();
+    mThreadLeaked = true;
   }
 
   MOZ_DIAGNOSTIC_ASSERT(joiningThread->IsOnCurrentThread());
+
   MarkCompleted();
+
   return NS_OK;
 }
 

xpcom/threads/nsThread.h

@@ -368,8 +368,8 @@ class nsThreadShutdownContext final : public nsIThreadShutdown {
                           nsThread* aJoiningThread)
       : mTerminatingThread(aTerminatingThread),
         mTerminatingPRThread(aTerminatingThread->GetPRThread()),
-        mJoiningThread(aJoiningThread,
-                       "nsThreadShutdownContext::mJoiningThread") {}
+        mJoiningThreadMutex("nsThreadShutdownContext::mJoiningThreadMutex"),
+        mJoiningThread(aJoiningThread) {}
 
   ~nsThreadShutdownContext() = default;
 
@@ -387,7 +387,9 @@ class nsThreadShutdownContext final : public nsIThreadShutdown {
   // The thread waiting for this thread to shut down. Will either be cleared by
   // the joining thread if `StopWaitingAndLeakThread` is called or by the
   // terminating thread upon exiting and notifying the joining thread.
-  mozilla::DataMutex<RefPtr<nsThread>> mJoiningThread;
+  mozilla::Mutex mJoiningThreadMutex;
+  RefPtr<nsThread> mJoiningThread GUARDED_BY(mJoiningThreadMutex);
+  bool mThreadLeaked GUARDED_BY(mJoiningThreadMutex) = false;
 };
 
 // This RAII class controls the duration of the associated nsThread's local