Lazily create the cleanup thread only when we encounter the scenario that would deadlock if cleanup were on the finalizer thread.

Add tests for this specific scenario.

Author: jkoritzinsky

src/coreclr/System.Private.CoreLib/src/System/Threading/Thread.CoreCLR.cs

@@ -441,11 +441,26 @@ private SafeWaitHandle GetJoinHandle()
         private volatile ManualResetEvent? _joinEvent;
         private SafeWaitHandle GetJoinHandle()
         {
-            ManualResetEvent joinEvent = Interlocked.CompareExchange(ref _joinEvent, new ManualResetEvent(false), null)!;
+            ManualResetEvent newEvent = new ManualResetEvent(false);
+            ManualResetEvent joinEvent = Interlocked.CompareExchange(ref _joinEvent, newEvent, null) ?? newEvent;
             return joinEvent.SafeWaitHandle;
         }
 #endif
 
+#if TARGET_UNIX || TARGET_BROWSER || TARGET_WASI
+        static partial void EnsureDetachedThreadCleanupThreadExistsCore()
+        {
+            if (!EnsureDetachedThreadCleanupThreadExistsInternal())
+            {
+                throw new OutOfMemoryException();
+            }
+        }
+
+        [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "ThreadNative_EnsureDetachedThreadCleanupThreadExists")]
+        [return: MarshalAs(UnmanagedType.Bool)]
+        private static partial bool EnsureDetachedThreadCleanupThreadExistsInternal();
+#endif // TARGET_UNIX || TARGET_BROWSER || TARGET_WASI
+
         /// <summary>
         /// Max value to be passed into <see cref="SpinWait(int)"/> for optimal delaying. This value is normalized to be
         /// appropriate for the processor.
@@ -563,6 +578,7 @@ private void OnThreadExiting()
             // Inform the wait subsystem that the thread is exiting. For instance, this would abandon any mutexes locked by
             // the thread.
             _waitInfo?.OnThreadExiting();
+            SetJoinHandle();
 #endif
         }
 

src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.Unix.cs

@@ -53,7 +53,7 @@ private static void OnThreadExit()
                 // the thread.
                 currentThread._waitInfo.OnThreadExiting();
                 StopThread(currentThread);
-                currentThread._stopped.Set();
+                SetJoinHandle(currentThread);
             }
         }
 

src/coreclr/vm/ceemain.cpp

@@ -936,10 +936,6 @@ void EEStartupHelper()
         FinalizerThread::FinalizerThreadCreate();
 #endif // TARGET_WINDOWS
 
-        // Start the thread that will clean up managed resources for any non-.NET created threads
-        // that exit.
-        ThreadCleanupThread::EnsureCleanupThreadExists();
-
 #ifdef FEATURE_PERFTRACING
         // Finish setting up rest of EventPipe - specifically enable SampleProfiler if it was requested at startup.
         // SampleProfiler needs to cooperate with the GC which hasn't fully finished setting up in the first part of the

src/coreclr/vm/comsynchronizable.cpp

@@ -723,6 +723,20 @@ extern "C" void QCALLTYPE ThreadNative_SpinWait(INT32 iterations)
     YieldProcessorNormalized(iterations);
 }
 
+extern "C" BOOL QCALLTYPE ThreadNative_EnsureDetachedThreadCleanupThreadExists()
+{
+    QCALL_CONTRACT;
+
+    BOOL ret = FALSE;
+    BEGIN_QCALL;
+
+    ret = ThreadCleanupThread::EnsureCleanupThreadExists();
+
+    END_QCALL;
+
+    return ret;
+}
+
 #ifdef TARGET_WINDOWS
 // This service can be called on unstarted and dead threads.  For unstarted ones, the
 // next wait will be interrupted.  For dead ones, this service quietly does nothing.

src/coreclr/vm/comsynchronizable.h

@@ -59,6 +59,7 @@ extern "C" INT32 QCALLTYPE ThreadNative_GetThreadState(QCall::ThreadHandle threa
 extern "C" void QCALLTYPE ThreadNative_SetWaitSleepJoinState(QCall::ThreadHandle thread);
 extern "C" void QCALLTYPE ThreadNative_ClearWaitSleepJoinState(QCall::ThreadHandle thread);
 extern "C" INT32 QCALLTYPE ThreadNative_ReentrantWaitAny(BOOL alertable, INT32 timeout, INT32 count, HANDLE *handles);
+extern "C" BOOL QCALLTYPE ThreadNative_EnsureDetachedThreadCleanupThreadExists();
 #ifdef TARGET_WINDOWS
 extern "C" void QCALLTYPE ThreadNative_Interrupt(QCall::ThreadHandle thread);
 extern "C" void QCALLTYPE ThreadNative_CheckForPendingInterrupt(QCall::ThreadHandle thread);

src/coreclr/vm/finalizerthread.cpp

@@ -146,6 +146,11 @@ static void DoExtraWorkForFinalizer(Thread* finalizerThread)
         SystemDomain::System()->ProcessDelayedUnloadLoaderAllocators();
     }
 
+    if (Thread::m_DetachCount > 0 && !ThreadCleanupThread::UsingExternalCleanupThread())
+    {
+        Thread::CleanupDetachedThreads();
+    }
+
     if (Thread::CleanupNeededForFinalizedThread())
     {
         Thread::CleanupFinalizedThreads();

src/coreclr/vm/object.h

@@ -1213,6 +1213,7 @@ class ThreadBaseObject : public Object
     OBJECTREF     m_StartHelper;
 #ifdef TARGET_UNIX
     OBJECTREF     m_WaitInfo;
+    OBJECTREF     m_joinEvent;
 #endif // TARGET_UNIX
 
     // The next field (m_InternalThread) is declared as IntPtr in the managed

src/coreclr/vm/qcallentrypoints.cpp

@@ -296,6 +296,7 @@ static const Entry s_QCall[] =
     DllImportEntry(ThreadNative_Abort)
     DllImportEntry(ThreadNative_ResetAbort)
     DllImportEntry(ThreadNative_SpinWait)
+    DllImportEntry(ThreadNative_EnsureDetachedThreadCleanupThreadExists)
 #ifdef TARGET_WINDOWS
     DllImportEntry(ThreadNative_CheckForPendingInterrupt)
     DllImportEntry(ThreadNative_Interrupt)

src/coreclr/vm/threads.cpp

@@ -6989,6 +6989,9 @@ namespace
         }
         CONTRACTL_END;
 
+        // If there's no threads to detach right now,
+        // go to sleep.
+        if (Thread::m_DetachCount == 0)
         {
             GCX_PREEMP();
 
@@ -7024,44 +7027,83 @@ namespace
     }
 }
 
-void
+BOOL
 ThreadCleanupThread::EnsureCleanupThreadExists()
 {
     CONTRACTL{
         THROWS;
         GC_TRIGGERS;
-        MODE_ANY;
+        MODE_PREEMPTIVE;
     } CONTRACTL_END;
 
-    ForeignThreadsToCleanUpEvent = new CLREvent();
-    ForeignThreadsToCleanUpEvent->CreateAutoEvent(FALSE);
+    if (ForeignThreadsToCleanUpEvent != nullptr)
+    {
+        // Another thread created the event first.
+        // Set the event in case another thread just signaled the finalizer
+        // right before we were called.
+        ForeignThreadsToCleanUpEvent->Set();
+        return TRUE;
+    }
+
+    NewHolder<CLREvent> tempEvent = new CLREvent();
+    // We may have threads waiting to clean up already, so create the event in the signaled state.
+    tempEvent->CreateAutoEvent(TRUE);
+
+    if (InterlockedCompareExchangeT(&ForeignThreadsToCleanUpEvent, tempEvent.GetValue(), nullptr) != nullptr)
+    {
+        // Another thread created the event first, just return.
+        return TRUE;
+    }
+
+    tempEvent.SuppressRelease();
 
-    Thread* pCleanupThread = SetupUnstartedThread();
+    Holder<Thread*,DoNothing<Thread*>,DeleteThread> pCleanupThread(SetupUnstartedThread());
 
-    if (pCleanupThread->CreateNewThread(0, &ForeignThreadCleanupThreadStart, pCleanupThread, W(".NET External Thread Cleanup Thread")))
+    // Don't block process shutdown on the cleanup thread.
+    pCleanupThread.GetValue()->SetBackground(TRUE);
+
+    if (pCleanupThread.GetValue()->CreateNewThread(0, &ForeignThreadCleanupThreadStart, pCleanupThread.GetValue(), W(".NET Detached Cleanup Thread")))
     {
-        DWORD dwRet = pCleanupThread->StartThread();
+        DWORD dwRet = pCleanupThread.GetValue()->StartThread();
 
         // When running under a user mode native debugger there is a race
         // between the moment we've created the thread (in CreateNewThread) and
         // the moment we resume it (in StartThread); the debugger may receive
         // the "ct" (create thread) notification, and it will attempt to
         // suspend/resume all threads in the process.  Now imagine the debugger
         // resumes this thread first, and only later does it try to resume the
-        // newly created thread (the finalizer thread).  In these conditions our
+        // newly created thread.  In these conditions our
         // call to ResumeThread may come before the debugger's call to ResumeThread
         // actually causing dwRet to equal 2.
         // We cannot use IsDebuggerPresent() in the condition below because the
         // debugger may have been detached between the time it got the notification
         // and the moment we execute the test below.
         _ASSERTE(dwRet == 1 || dwRet == 2);
+        pCleanupThread.SuppressRelease();
+        return TRUE;
     }
+
+    return FALSE;
 }
 void
 ThreadCleanupThread::SetHasThreadsToCleanUp()
 {
-    _ASSERT(ForeignThreadsToCleanUpEvent != nullptr);
-    ForeignThreadsToCleanUpEvent->Set();
+    // If we don't have a dedicated cleanup thread,
+    // we will cleanup detached threads on the finalizer thread.
+    if (ForeignThreadsToCleanUpEvent == nullptr)
+    {
+        FinalizerThread::EnableFinalization();
+    }
+    else
+    {
+        ForeignThreadsToCleanUpEvent->Set();
+    }
+}
+
+bool
+ThreadCleanupThread::UsingExternalCleanupThread()
+{
+    return ForeignThreadsToCleanUpEvent != nullptr;
 }
 #endif // DACCESS_COMPILE
 

src/coreclr/vm/threads.h

@@ -5486,7 +5486,8 @@ class StackWalkerWalkingThreadHolder
 class ThreadCleanupThread
 {
     public:
-    static void EnsureCleanupThreadExists();
+    static bool UsingExternalCleanupThread();
+    static BOOL EnsureCleanupThreadExists();
     static void SetHasThreadsToCleanUp();
 };