From ddb3007e4eac99cc107b807a8338674bee3cf670 Mon Sep 17 00:00:00 2001
From: Jens Stutte <jstutte@mozilla.com>
Date: Wed, 15 Jun 2022 18:15:01 +0000
Subject: [PATCH] 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 | 31 +++++++++++++++++++++++--------
 xpcom/threads/nsThread.h   |  8 +++++---
 2 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/xpcom/threads/nsThread.cpp b/xpcom/threads/nsThread.cpp
index e8c555f3ff8b..a8b17eacb47b 100644
--- a/xpcom/threads/nsThread.cpp
+++ b/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;
 }
 
diff --git a/xpcom/threads/nsThread.h b/xpcom/threads/nsThread.h
index 82e3cff039a1..b4b0c3023cab 100644
--- a/xpcom/threads/nsThread.h
+++ b/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