Bug 1599922 clear PRThread references before the PRThread is deleted r=froydnj

Virtual thread references are used for IsOnCurrentThread(), which would
spuriously return true when the dangling pointer happened to match that of a
new PRThread.

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

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

xpcom/threads/ThreadEventTarget.cpp

@@ -96,6 +96,8 @@ void ThreadEventTarget::SetCurrentThread() {
   mVirtualThread = GetCurrentVirtualThread();
 }
 
+void ThreadEventTarget::ClearCurrentThread() { mVirtualThread = nullptr; }
+
 NS_IMPL_ISUPPORTS(ThreadEventTarget, nsIEventTarget, nsISerialEventTarget)
 
 NS_IMETHODIMP

xpcom/threads/ThreadEventTarget.h

@@ -31,6 +31,8 @@ class ThreadEventTarget final : public nsISerialEventTarget {
   // Sets the thread for which IsOnCurrentThread returns true to the current
   // thread.
   void SetCurrentThread();
+  // Call ClearCurrentThread() before the PRThread is deleted on thread join.
+  void ClearCurrentThread();
 
   size_t SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const {
     size_t n = 0;

xpcom/threads/nsIEventTarget.idl

@@ -9,6 +9,7 @@
 %{C++
 #include "nsCOMPtr.h"
 #include "mozilla/AlreadyAddRefed.h"
+#include "mozilla/Atomics.h"
 %}
 
 native alreadyAddRefed_nsIRunnable(already_AddRefed<nsIRunnable>);
@@ -94,7 +95,7 @@ public:
   bool IsOnCurrentThread();
 
 protected:
-  PRThread* mVirtualThread;
+  mozilla::Atomic<PRThread*> mVirtualThread;
 
   nsIEventTarget() : mVirtualThread(nullptr) {}
   %}

xpcom/threads/nsThread.cpp

@@ -241,6 +241,7 @@ struct nsThreadShutdownContext {
                           NotNull<nsThread*> aJoiningThread,
                           bool aAwaitingShutdownAck)
       : mTerminatingThread(aTerminatingThread),
+        mTerminatingPRThread(aTerminatingThread->GetPRThread()),
         mJoiningThread(aJoiningThread),
         mAwaitingShutdownAck(aAwaitingShutdownAck),
         mIsMainThreadJoining(NS_IsMainThread()) {
@@ -250,6 +251,7 @@ struct nsThreadShutdownContext {
 
   // NB: This will be the last reference.
   NotNull<RefPtr<nsThread>> mTerminatingThread;
+  PRThread* const mTerminatingPRThread;
   NotNull<nsThread*> MOZ_UNSAFE_REF(
       "Thread manager is holding reference to joining thread") mJoiningThread;
   bool mAwaitingShutdownAck;
@@ -504,6 +506,10 @@ void nsThread::ThreadFunc(void* aArg) {
   FreeTraceInfo();
 #endif
 
+  // The PRThread will be deleted in PR_JoinThread(), so clear references.
+  self->mThread = nullptr;
+  self->mVirtualThread = nullptr;
+  self->mEventTarget->ClearCurrentThread();
   NS_RELEASE(self);
 }
 
@@ -764,8 +770,10 @@ nsThread::IsOnCurrentThread(bool* aResult) {
 
 NS_IMETHODIMP_(bool)
 nsThread::IsOnCurrentThreadInfallible() {
-  // Rely on mVirtualThread being correct.
-  MOZ_CRASH("IsOnCurrentThreadInfallible should never be called on nsIThread");
+  // This method is only going to be called if `mVirtualThread` is null, which
+  // only happens when the thread has exited the event loop.  Therefore, when
+  // we are called, we can never be on this thread.
+  return false;
 }
 
 //-----------------------------------------------------------------------------
@@ -866,7 +874,6 @@ nsThreadShutdownContext* nsThread::ShutdownInternal(bool aSync) {
 void nsThread::ShutdownComplete(NotNull<nsThreadShutdownContext*> aContext) {
   MOZ_ASSERT(mEvents);
   MOZ_ASSERT(mEventTarget);
-  MOZ_ASSERT(mThread);
   MOZ_ASSERT(aContext->mTerminatingThread == this);
 
   MaybeRemoveFromThreadList();
@@ -879,9 +886,8 @@ void nsThread::ShutdownComplete(NotNull<nsThreadShutdownContext*> aContext) {
   }
 
   // Now, it should be safe to join without fear of dead-locking.
-
-  PR_JoinThread(mThread);
-  mThread = nullptr;
+  PR_JoinThread(aContext->mTerminatingPRThread);
+  MOZ_ASSERT(!mThread);
 
 #ifdef DEBUG
   nsCOMPtr<nsIThreadObserver> obs = mEvents->GetObserver();