Ensure consistency between ThreadState.Stopped APIs that check if a thread is dead (#122568)

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Author: jkoritzinsky

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

@@ -63,7 +63,10 @@ public sealed partial class Thread
         // but those types of changes may race with the reset anyway, so this field doesn't need to be synchronized.
         private bool _mayNeedResetForThreadPool;
 
-        // Set in unmanaged and read in managed code.
+        // This is set in two places:
+        // For threads started with Thread.Start: Set in managed code as the thread is exiting.
+        // For external threads that attach to the runtime: Set in unmanaged code as part of thread detach.
+        // This is only read in managed code.
         private bool _isDead;
         private bool _isThreadPool;
 
@@ -290,6 +293,11 @@ public ThreadState ThreadState
         {
             get
             {
+                if (_isDead)
+                {
+                    return ThreadState.Stopped;
+                }
+
                 var state = (ThreadState)GetThreadState(GetNativeHandle());
                 GC.KeepAlive(this);
                 return state;
@@ -324,16 +332,6 @@ internal void ClearWaitSleepJoinState()
         [SuppressGCTransition]
         private static partial void ClearWaitSleepJoinState(ThreadHandle t);
 
-        [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "ThreadNative_ReportDead")]
-        [SuppressGCTransition]
-        private static partial void ReportDead(ThreadHandle t);
-
-        internal void NotifyThreadDeath()
-        {
-            ReportDead(GetNativeHandle());
-            GC.KeepAlive(this);
-        }
-
         /// <summary>
         /// An unstarted thread can be marked to indicate that it will host a
         /// single-threaded or multi-threaded apartment.
@@ -568,16 +566,15 @@ WaitSubsystem.ThreadWaitInfo AllocateWaitInfo()
         }
 #endif
 
-#pragma warning disable CA1822 // Member 'OnThreadExiting' does not access instance data and can be marked as static
         private void OnThreadExiting()
-#pragma warning restore CA1822 // Member 'OnThreadExiting' does not access instance data and can be marked as static
         {
-#if TARGET_UNIX || TARGET_BROWSER || TARGET_WASI
-            // Update the native side to report that the thread is dead.
-            // We do this before OnThreadExiting and SetJoinHandle so that any threads
-            // waiting on this thread to end will correctly see that it is stopped
+            // Consider this managed thread as dead.
+            // The unmanaged thread is still alive, but will die soon, after cleaning up some state.
+            // We set _isDead = true before calling _waitInfo?.OnThreadExiting() and SetJoinHandle()
+            // so that any threads waiting on this thread to end will correctly see that it is stopped
             // when we set the join handle.
-            NotifyThreadDeath();
+            _isDead = true;
+#if TARGET_UNIX || TARGET_BROWSER || TARGET_WASI
             // Inform the wait subsystem that the thread is exiting. For instance, this would abandon any mutexes locked by
             // the thread.
             _waitInfo?.OnThreadExiting();

src/coreclr/vm/comsynchronizable.cpp

@@ -394,14 +394,8 @@ extern "C" INT32 QCALLTYPE ThreadNative_GetThreadState(QCall::ThreadHandle threa
     // grab a snapshot
     Thread::ThreadState state = thread->GetSnapshotState();
 
-    // If the thread is dead, just report that its dead.
-    // It's possible that the thread may still be in the final bits of shutting down,
-    // but for the purposes of the managed API surface, it should be reported
-    // as stopped.
     if (state & Thread::TS_Dead)
-    {
-        return ThreadNative::ThreadStopped;
-    }
+        res |= ThreadNative::ThreadStopped;
 
     if (state & Thread::TS_Background)
         res |= ThreadNative::ThreadBackground;
@@ -446,18 +440,6 @@ extern "C" void QCALLTYPE ThreadNative_ClearWaitSleepJoinState(QCall::ThreadHand
     thread->ResetThreadStateNC(Thread::TSNC_DebuggerSleepWaitJoin);
 }
 
-extern "C" void QCALLTYPE ThreadNative_ReportDead(QCall::ThreadHandle thread)
-{
-    CONTRACTL
-    {
-        QCALL_CHECK_NO_GC_TRANSITION;
-        PRECONDITION(thread != NULL);
-    }
-    CONTRACTL_END;
-
-    thread->SetThreadState(Thread::TS_ReportDead);
-}
-
 #ifdef FEATURE_COMINTEROP_APARTMENT_SUPPORT
 
 // Return whether the thread hosts an STA, is a member of the MTA or is not

src/coreclr/vm/comsynchronizable.h

@@ -58,7 +58,6 @@ extern "C" void QCALLTYPE ThreadNative_Initialize(QCall::ObjectHandleOnStack t);
 extern "C" INT32 QCALLTYPE ThreadNative_GetThreadState(QCall::ThreadHandle thread);
 extern "C" void QCALLTYPE ThreadNative_SetWaitSleepJoinState(QCall::ThreadHandle thread);
 extern "C" void QCALLTYPE ThreadNative_ClearWaitSleepJoinState(QCall::ThreadHandle thread);
-extern "C" void QCALLTYPE ThreadNative_ReportDead(QCall::ThreadHandle thread);
 extern "C" INT32 QCALLTYPE ThreadNative_ReentrantWaitAny(BOOL alertable, INT32 timeout, INT32 count, HANDLE *handles);
 #ifdef TARGET_WINDOWS
 extern "C" void QCALLTYPE ThreadNative_Interrupt(QCall::ThreadHandle thread);

src/coreclr/vm/qcallentrypoints.cpp

@@ -296,7 +296,6 @@ static const Entry s_QCall[] =
     DllImportEntry(ThreadNative_GetThreadState)
     DllImportEntry(ThreadNative_SetWaitSleepJoinState)
     DllImportEntry(ThreadNative_ClearWaitSleepJoinState)
-    DllImportEntry(ThreadNative_ReportDead)
     DllImportEntry(ThreadNative_ReentrantWaitAny)
 #ifdef FEATURE_COMINTEROP_APARTMENT_SUPPORT
     DllImportEntry(ThreadNative_GetApartmentState)