Use FLS detach callback as a thread termination notification. Another try. (#112809)

* Use FLS detach as thread termination notification on windows.

* use _ASSERTE_ALL_BUILDS

* one more case

* InitFlsSlot throws per convention used in threading initialization

* OsDetachThread could be void

* Update src/coreclr/vm/ceemain.cpp

* ensure CoInitialize

* Asserts to fail deterministically.

* comments

* scope the failfast to Windows and tweak the comments a bit.

* better assert

* Apply suggestions from code review

Co-authored-by: Jan Kotas <[email protected]>

* Undo unnecessary finalizer thread changes

* reverse the failfast condition

* handle destruction of unattached threads

* PR feedback

Co-authored-by: Jan Kotas <[email protected]>

* Update src/coreclr/vm/ceemain.cpp

* set g_fComStarted as soon as we call CoInitialize

* Naming nit

* fix a typpo + better comment

---------

Co-authored-by: Jan Kotas <[email protected]>

Author: VSadov

src/coreclr/nativeaot/Runtime/threadstore.cpp

@@ -157,7 +157,7 @@ void ThreadStore::DetachCurrentThread()
     }
 
     // Unregister from OS notifications
-    // This can return false if detach notification is spurious and does not belong to this thread.
+    // This can return false if a thread did not register for OS notification.
     if (!PalDetachThread(pDetachingThread))
     {
         return;

src/coreclr/vm/ceemain.cpp

@@ -872,6 +872,10 @@ void EEStartupHelper()
         }
 #endif
 
+        // This isn't done as part of InitializeGarbageCollector() above because
+        // debugger must be initialized before creating EE thread objects 
+        FinalizerThread::FinalizerThreadCreate();
+
         InitPreStubManager();
 
 #ifdef FEATURE_COMINTEROP
@@ -909,10 +913,6 @@ void EEStartupHelper()
 #endif // FEATURE_PERFTRACING
         GenAnalysis::Initialize();
 
-        // This isn't done as part of InitializeGarbageCollector() above because thread
-        // creation requires AppDomains to have been set up.
-        FinalizerThread::FinalizerThreadCreate();
-
         // Now we really have fully initialized the garbage collector
         SetGarbageCollectorFullyInitialized();
 
@@ -964,6 +964,12 @@ void EEStartupHelper()
                                                 g_MiniMetaDataBuffMaxSize, MEM_COMMIT, PAGE_READWRITE);
 #endif // FEATURE_MINIMETADATA_IN_TRIAGEDUMPS
 
+#ifdef TARGET_WINDOWS
+        // By now finalizer thread should have initialized FLS slot for thread cleanup notifications.
+        // And ensured that COM is initialized (must happen before allocating FLS slot).
+        // Make sure that this was done.
+        FinalizerThread::WaitForFinalizerThreadStart();
+#endif
         g_fEEStarted = TRUE;
         g_EEStartupStatus = S_OK;
         hr = S_OK;
@@ -1684,6 +1690,135 @@ BOOL STDMETHODCALLTYPE EEDllMain( // TRUE on success, FALSE on error.
 
 #endif // !defined(CORECLR_EMBEDDED)
 
+static void RuntimeThreadShutdown(void* thread)
+{
+    Thread* pThread = (Thread*)thread;
+    _ASSERTE(pThread == GetThreadNULLOk());
+
+    if (pThread)
+    {
+#ifdef FEATURE_COMINTEROP
+        // reset the CoInitialize state
+        // so we don't call CoUninitialize during thread detach
+        pThread->ResetCoInitialized();
+#endif // FEATURE_COMINTEROP
+        // For case where thread calls ExitThread directly, we need to reset the
+        // frame pointer. Otherwise stackwalk would AV. We need to do it in cooperative mode.
+        // We need to set m_GCOnTransitionsOK so this thread won't trigger GC when toggle GC mode
+        if (pThread->m_pFrame != FRAME_TOP)
+        {
+#ifdef _DEBUG
+            pThread->m_GCOnTransitionsOK = FALSE;
+#endif
+            GCX_COOP_NO_DTOR();
+            pThread->m_pFrame = FRAME_TOP;
+            GCX_COOP_NO_DTOR_END();
+        }
+
+        pThread->DetachThread(TRUE);
+    }
+    else
+    {
+        // Since we don't actually cleanup the TLS data along this path, verify that it is already cleaned up
+        AssertThreadStaticDataFreed();
+    }
+
+    ThreadDetaching();
+}
+
+#ifdef TARGET_WINDOWS
+
+// Index for the fiber local storage of the attached thread pointer
+static uint32_t g_flsIndex = FLS_OUT_OF_INDEXES;
+
+#define FLS_STATE_CLEAR 0
+#define FLS_STATE_ARMED 1
+#define FLS_STATE_INVOKED 2
+
+static __declspec(thread) byte t_flsState;
+
+// This is called when each *fiber* is destroyed. When the home fiber of a thread is destroyed,
+// it means that the thread itself is destroyed.
+// Since we receive that notification outside of the Loader Lock, it allows us to safely acquire
+// the ThreadStore lock in the RuntimeThreadShutdown.
+static void __stdcall FiberDetachCallback(void* lpFlsData)
+{
+    _ASSERTE(g_flsIndex != FLS_OUT_OF_INDEXES);
+    _ASSERTE(lpFlsData);
+
+    if (t_flsState == FLS_STATE_ARMED)
+    {
+        RuntimeThreadShutdown(lpFlsData);
+    }
+
+    t_flsState = FLS_STATE_INVOKED;
+}
+
+void InitFlsSlot()
+{
+    // We use fiber detach callbacks to run our thread shutdown code because the fiber detach
+    // callback is made without the OS loader lock
+    g_flsIndex = FlsAlloc(FiberDetachCallback);
+    if (g_flsIndex == FLS_OUT_OF_INDEXES)
+    {
+        COMPlusThrowWin32();
+    }
+}
+
+// Register the thread with OS to be notified when thread is about to be destroyed
+// It fails fast if a different thread was already registered with the current fiber.
+// Parameters:
+//  thread        - thread to attach
+static void OsAttachThread(void* thread)
+{
+    if (t_flsState == FLS_STATE_INVOKED)
+    {
+        _ASSERTE_ALL_BUILDS(!"Attempt to execute managed code after the .NET runtime thread state has been destroyed.");
+    }
+
+    t_flsState = FLS_STATE_ARMED;
+
+    // Associate the current fiber with the current thread.  This makes the current fiber the thread's "home"
+    // fiber.  This fiber is the only fiber allowed to execute managed code on this thread.  When this fiber
+    // is destroyed, we consider the thread to be destroyed.
+    _ASSERTE(thread != NULL);
+    FlsSetValue(g_flsIndex, thread);
+}
+
+// Detach thread from OS notifications.
+// It fails fast if some other thread value was attached to the current fiber.
+// Parameters:
+//  thread        - thread to detach
+void OsDetachThread(void* thread)
+{
+    ASSERT(g_flsIndex != FLS_OUT_OF_INDEXES);
+    void* threadFromCurrentFiber = FlsGetValue(g_flsIndex);
+
+    if (threadFromCurrentFiber == NULL)
+    {
+        // Thread is not attached.
+        // This could come from DestroyThread called when refcount reaches 0
+        // and the thread may have already been detached or never attached.
+        // We leave t_flsState as-is to keep track whether our callback has been called.
+        return;
+    }
+
+    if (threadFromCurrentFiber != thread)
+    {
+        _ASSERTE_ALL_BUILDS(!"Detaching a thread from the wrong fiber");
+    }
+
+    // Leave the existing FLS value, to keep the callback "armed" so that we could observe the termination callback.
+    // After that we will not allow to attach as we will no longer be able to clean up.
+    t_flsState = FLS_STATE_CLEAR;
+}
+
+void EnsureTlsDestructionMonitor()
+{
+    OsAttachThread(GetThread());
+}
+
+#else
 struct TlsDestructionMonitor
 {
     bool m_activated = false;
@@ -1697,36 +1832,7 @@ struct TlsDestructionMonitor
     {
         if (m_activated)
         {
-            Thread* thread = GetThreadNULLOk();
-            if (thread)
-            {
-#ifdef FEATURE_COMINTEROP
-                // reset the CoInitialize state
-                // so we don't call CoUninitialize during thread detach
-                thread->ResetCoInitialized();
-#endif // FEATURE_COMINTEROP
-                // For case where thread calls ExitThread directly, we need to reset the
-                // frame pointer. Otherwise stackwalk would AV. We need to do it in cooperative mode.
-                // We need to set m_GCOnTransitionsOK so this thread won't trigger GC when toggle GC mode
-                if (thread->m_pFrame != FRAME_TOP)
-                {
-#ifdef _DEBUG
-                    thread->m_GCOnTransitionsOK = FALSE;
-#endif
-                    GCX_COOP_NO_DTOR();
-                    thread->m_pFrame = FRAME_TOP;
-                    GCX_COOP_NO_DTOR_END();
-                }
-
-                thread->DetachThread(TRUE);
-            }
-            else
-            {
-                // Since we don't actually cleanup the TLS data along this path, verify that it is already cleaned up
-                AssertThreadStaticDataFreed();
-            }
-
-            ThreadDetaching();
+            RuntimeThreadShutdown(GetThreadNULLOk());
         }
     }
 };
@@ -1740,6 +1846,8 @@ void EnsureTlsDestructionMonitor()
     tls_destructionMonitor.Activate();
 }
 
+#endif
+
 #ifdef DEBUGGING_SUPPORTED
 //
 // InitializeDebugger initialized the Runtime-side CLR Debugging Services

src/coreclr/vm/ceemain.h

@@ -46,6 +46,10 @@ void ForceEEShutdown(ShutdownCompleteAction sca = SCA_ExitProcessWhenShutdownCom
 void ThreadDetaching();
 
 void EnsureTlsDestructionMonitor();
+#ifdef TARGET_WINDOWS
+void InitFlsSlot();
+void OsDetachThread(void* thread);
+#endif
 
 void SetLatchedExitCode (INT32 code);
 INT32 GetLatchedExitCode (void);

src/coreclr/vm/finalizerthread.cpp

@@ -380,6 +380,20 @@ DWORD WINAPI FinalizerThread::FinalizerThreadStart(void *args)
 
     LOG((LF_GC, LL_INFO10, "Finalizer thread starting...\n"));
 
+#ifdef TARGET_WINDOWS
+#ifdef FEATURE_COMINTEROP
+    // Making finalizer thread MTA early ensures that COM is initialized before we initialize our thread
+    // termination callback.
+    ::CoInitializeEx(NULL, COINIT_MULTITHREADED);
+    g_fComStarted = true;
+#endif
+
+    InitFlsSlot();
+
+    // handshake with EE initialization, as now we can attach Thread objects to native threads.
+    hEventFinalizerDone->Set();
+#endif
+
     s_FinalizerThreadOK = GetFinalizerThread()->HasStarted();
 
     _ASSERTE(s_FinalizerThreadOK);
@@ -491,6 +505,15 @@ void FinalizerThread::SignalFinalizationDone(int observedFullGcCount)
     hEventFinalizerDone->Set();
 }
 
+void FinalizerThread::WaitForFinalizerThreadStart()
+{
+    // this should be only called during EE startup
+    _ASSERTE(!g_fEEStarted);
+
+    hEventFinalizerDone->Wait(INFINITE,FALSE);
+    hEventFinalizerDone->Reset();
+}
+
 // Wait for the finalizer thread to complete one pass.
 void FinalizerThread::FinalizerThreadWait()
 {

src/coreclr/vm/finalizerthread.h

@@ -65,6 +65,8 @@ class FinalizerThread
         }
     }
 
+    static void WaitForFinalizerThreadStart();
+
     static void FinalizerThreadWait();
 
     static void SignalFinalizationDone(int observedFullGcCount);

src/coreclr/vm/interoputil.cpp

@@ -1404,22 +1404,8 @@ VOID EnsureComStarted(BOOL fCoInitCurrentThread)
     }
     CONTRACTL_END;
 
-    if (g_fComStarted == FALSE)
-    {
-        FinalizerThread::GetFinalizerThread()->SetRequiresCoInitialize();
-
-        // Attempt to set the thread's apartment model (to MTA by default). May not
-        // succeed (if someone beat us to the punch). That doesn't matter (since
-        // CLR objects are now apartment agile), we only care that a CoInitializeEx
-        // has been performed on this thread by us.
-        if (fCoInitCurrentThread)
-            GetThread()->SetApartment(Thread::AS_InMTA);
-
-        // set the finalizer event
-        FinalizerThread::EnableFinalization();
-
-        g_fComStarted = TRUE;
-    }
+    // COM is expected to be started on finalizer thread during startup
+    _ASSERTE(g_fComStarted);
 }
 
 HRESULT EnsureComStartedNoThrow(BOOL fCoInitCurrentThread)
@@ -1436,15 +1422,8 @@ HRESULT EnsureComStartedNoThrow(BOOL fCoInitCurrentThread)
 
     HRESULT hr = S_OK;
 
-    if (!g_fComStarted)
-    {
-        GCX_COOP();
-        EX_TRY
-        {
-            EnsureComStarted(fCoInitCurrentThread);
-        }
-        EX_CATCH_HRESULT(hr);
-    }
+    // COM is expected to be started on finalizer thread during startup
+    _ASSERTE(g_fComStarted);
 
     return hr;
 }

src/coreclr/vm/threads.cpp

@@ -353,13 +353,23 @@ void SetThread(Thread* t)
 {
     LIMITED_METHOD_CONTRACT
 
+    Thread* origThread = gCurrentThreadInfo.m_pThread;
     gCurrentThreadInfo.m_pThread = t;
     if (t != NULL)
     {
+        _ASSERTE(origThread == NULL);
         InitializeCurrentThreadsStaticData(t);
         EnsureTlsDestructionMonitor();
         t->InitRuntimeThreadLocals();
     }
+#ifdef TARGET_WINDOWS
+    else if (origThread != NULL)
+    {
+        // Unregister from OS notifications
+        // This can return false if a thread did not register for OS notification.
+        OsDetachThread(origThread);
+    }
+#endif
 
     // Clear or set the app domain to the one domain based on if the thread is being nulled out or set
     gCurrentThreadInfo.m_pAppDomain = t == NULL ? NULL : AppDomain::GetCurrentDomain();
@@ -865,7 +875,7 @@ void DestroyThread(Thread *th)
 // Public function: DetachThread()
 // Marks the thread as needing to be destroyed, but doesn't destroy it yet.
 //-------------------------------------------------------------------------
-HRESULT Thread::DetachThread(BOOL fDLLThreadDetach)
+HRESULT Thread::DetachThread(BOOL inTerminationCallback)
 {
     // !!! Can not use contract here.
     // !!! Contract depends on Thread object for GC_TRIGGERS.
@@ -890,9 +900,9 @@ HRESULT Thread::DetachThread(BOOL fDLLThreadDetach)
         pErrorInfo->Release();
     }
 
-    // Revoke our IInitializeSpy registration only if we are not in DLL_THREAD_DETACH
+    // Revoke our IInitializeSpy registration only if we are not in a thread termination callback
     // (COM will do it or may have already done it automatically in that case).
-    if (!fDLLThreadDetach)
+    if (!inTerminationCallback)
     {
         RevokeApartmentSpy();
     }

src/coreclr/vm/threads.h

@@ -678,7 +678,7 @@ class Thread
     };
 
 public:
-    HRESULT DetachThread(BOOL fDLLThreadDetach);
+    HRESULT DetachThread(BOOL inTerminationCallback);
 
     void SetThreadState(ThreadState ts)
     {