From f00de807f154ee77e509f6242bba6e4404fb6415 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Tue, 10 Dec 2024 10:52:35 -0800 Subject: [PATCH 1/6] Use FLS detach as thread termination notification on windows. --- src/coreclr/nativeaot/Runtime/threadstore.cpp | 2 +- src/coreclr/vm/ceemain.cpp | 157 ++++++++++++++---- src/coreclr/vm/ceemain.h | 4 + src/coreclr/vm/threads.cpp | 17 ++ 4 files changed, 149 insertions(+), 31 deletions(-) diff --git a/src/coreclr/nativeaot/Runtime/threadstore.cpp b/src/coreclr/nativeaot/Runtime/threadstore.cpp index c2b42491a387d9..c03907e4eed665 100644 --- a/src/coreclr/nativeaot/Runtime/threadstore.cpp +++ b/src/coreclr/nativeaot/Runtime/threadstore.cpp @@ -162,7 +162,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; diff --git a/src/coreclr/vm/ceemain.cpp b/src/coreclr/vm/ceemain.cpp index 2784196ab2e959..dfc7b0b7a93940 100644 --- a/src/coreclr/vm/ceemain.cpp +++ b/src/coreclr/vm/ceemain.cpp @@ -1701,6 +1701,130 @@ 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; + +// 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) +{ + ASSERT(g_flsIndex != FLS_OUT_OF_INDEXES); + ASSERT(lpFlsData == FlsGetValue(g_flsIndex)); + + if (lpFlsData != NULL) + { + // The current fiber is the home fiber of a thread, so the thread is shutting down + RuntimeThreadShutdown(lpFlsData); + } +} + +bool 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) + { + return false; + } + + return true; +} + +// 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) +{ + void* threadFromCurrentFiber = FlsGetValue(g_flsIndex); + + if (threadFromCurrentFiber != NULL) + { + _ASSERTE(!"Multiple threads encountered from a single fiber"); + COMPlusThrowWin32(); + } + + // 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. + 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 +// Return: +// true if the thread was detached, false if there was no attached thread +bool OsDetachThread(void* thread) +{ + ASSERT(g_flsIndex != FLS_OUT_OF_INDEXES); + void* threadFromCurrentFiber = FlsGetValue(g_flsIndex); + + if (threadFromCurrentFiber == NULL) + { + // we've seen this thread, but not this fiber. It must be a "foreign" fiber that was + // borrowing this thread. + return false; + } + + if (threadFromCurrentFiber != thread) + { + _ASSERTE(!"Detaching a thread from the wrong fiber"); + COMPlusThrowWin32(); + } + + FlsSetValue(g_flsIndex, NULL); + return true; +} + +void EnsureTlsDestructionMonitor() +{ + OsAttachThread(GetThread()); +} + +#else struct TlsDestructionMonitor { bool m_activated = false; @@ -1714,36 +1838,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()); } } }; @@ -1757,6 +1852,8 @@ void EnsureTlsDestructionMonitor() tls_destructionMonitor.Activate(); } +#endif + #ifdef DEBUGGING_SUPPORTED // // InitializeDebugger initialized the Runtime-side COM+ Debugging Services diff --git a/src/coreclr/vm/ceemain.h b/src/coreclr/vm/ceemain.h index 1404a5a04237ff..fec164ea3920b7 100644 --- a/src/coreclr/vm/ceemain.h +++ b/src/coreclr/vm/ceemain.h @@ -46,6 +46,10 @@ void ForceEEShutdown(ShutdownCompleteAction sca = SCA_ExitProcessWhenShutdownCom void ThreadDetaching(); void EnsureTlsDestructionMonitor(); +#ifdef TARGET_WINDOWS +bool InitFlsSlot(); +bool OsDetachThread(void* thread); +#endif void SetLatchedExitCode (INT32 code); INT32 GetLatchedExitCode (void); diff --git a/src/coreclr/vm/threads.cpp b/src/coreclr/vm/threads.cpp index e89b6c7d94c1d0..bc11c0e690964f 100644 --- a/src/coreclr/vm/threads.cpp +++ b/src/coreclr/vm/threads.cpp @@ -353,6 +353,7 @@ void SetThread(Thread* t) { LIMITED_METHOD_CONTRACT + Thread* origThread = gCurrentThreadInfo.m_pThread; gCurrentThreadInfo.m_pThread = t; if (t != NULL) { @@ -360,6 +361,14 @@ void SetThread(Thread* 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(); @@ -1039,6 +1048,14 @@ void InitThreadManager() } CONTRACTL_END; +#ifdef TARGET_WINDOWS + if (!InitFlsSlot()) + { + _ASSERTE(!"Initialization of a FLS slot failed."); + COMPlusThrowWin32(); + } +#endif + // All patched helpers should fit into one page. // If you hit this assert on retail build, there is most likely problem with BBT script. _ASSERTE_ALL_BUILDS((BYTE*)JIT_PatchedCodeLast - (BYTE*)JIT_PatchedCodeStart > (ptrdiff_t)0); From c98ca5cdc91fbf8dfbedad342f07bb55a5f7a71a Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Tue, 10 Dec 2024 15:33:09 -0800 Subject: [PATCH 2/6] use _ASSERTE_ALL_BUILDS --- src/coreclr/vm/ceemain.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/coreclr/vm/ceemain.cpp b/src/coreclr/vm/ceemain.cpp index dfc7b0b7a93940..7a16565bef0d74 100644 --- a/src/coreclr/vm/ceemain.cpp +++ b/src/coreclr/vm/ceemain.cpp @@ -1781,8 +1781,7 @@ static void OsAttachThread(void* thread) if (threadFromCurrentFiber != NULL) { - _ASSERTE(!"Multiple threads encountered from a single fiber"); - COMPlusThrowWin32(); + _ASSERTE_ALL_BUILDS(!"Multiple threads encountered from a single fiber"); } // Associate the current fiber with the current thread. This makes the current fiber the thread's "home" @@ -1811,8 +1810,7 @@ bool OsDetachThread(void* thread) if (threadFromCurrentFiber != thread) { - _ASSERTE(!"Detaching a thread from the wrong fiber"); - COMPlusThrowWin32(); + _ASSERTE_ALL_BUILDS(!"Detaching a thread from the wrong fiber"); } FlsSetValue(g_flsIndex, NULL); From 16b83c24275083bd036f0c065ec1c755dbd76af3 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Tue, 10 Dec 2024 15:34:46 -0800 Subject: [PATCH 3/6] one more case --- src/coreclr/vm/threads.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/coreclr/vm/threads.cpp b/src/coreclr/vm/threads.cpp index bc11c0e690964f..849a891f03bcda 100644 --- a/src/coreclr/vm/threads.cpp +++ b/src/coreclr/vm/threads.cpp @@ -1051,8 +1051,7 @@ void InitThreadManager() #ifdef TARGET_WINDOWS if (!InitFlsSlot()) { - _ASSERTE(!"Initialization of a FLS slot failed."); - COMPlusThrowWin32(); + _ASSERTE_ALL_BUILDS(!"Initialization of a FLS slot failed."); } #endif From 8a9f1fd7893e08de45a8a137bb2c2b539564d7e6 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Tue, 10 Dec 2024 16:51:07 -0800 Subject: [PATCH 4/6] InitFlsSlot throws per convention used in threading initialization --- src/coreclr/vm/ceemain.cpp | 7 +++---- src/coreclr/vm/ceemain.h | 2 +- src/coreclr/vm/threads.cpp | 5 +---- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/coreclr/vm/ceemain.cpp b/src/coreclr/vm/ceemain.cpp index 7a16565bef0d74..4d33044b674cbd 100644 --- a/src/coreclr/vm/ceemain.cpp +++ b/src/coreclr/vm/ceemain.cpp @@ -1758,17 +1758,16 @@ static void __stdcall FiberDetachCallback(void* lpFlsData) } } -bool InitFlsSlot() +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) { - return false; + _ASSERTE(!"Initialization of an FLS slot failed."); + COMPlusThrowWin32(); } - - return true; } // Register the thread with OS to be notified when thread is about to be destroyed diff --git a/src/coreclr/vm/ceemain.h b/src/coreclr/vm/ceemain.h index fec164ea3920b7..69ba92d692d6d2 100644 --- a/src/coreclr/vm/ceemain.h +++ b/src/coreclr/vm/ceemain.h @@ -47,7 +47,7 @@ void ThreadDetaching(); void EnsureTlsDestructionMonitor(); #ifdef TARGET_WINDOWS -bool InitFlsSlot(); +void InitFlsSlot(); bool OsDetachThread(void* thread); #endif diff --git a/src/coreclr/vm/threads.cpp b/src/coreclr/vm/threads.cpp index 849a891f03bcda..7ffb9f4746d94a 100644 --- a/src/coreclr/vm/threads.cpp +++ b/src/coreclr/vm/threads.cpp @@ -1049,10 +1049,7 @@ void InitThreadManager() CONTRACTL_END; #ifdef TARGET_WINDOWS - if (!InitFlsSlot()) - { - _ASSERTE_ALL_BUILDS(!"Initialization of a FLS slot failed."); - } + InitFlsSlot(); #endif // All patched helpers should fit into one page. From 4b8cd71789f39c0258b58c654e73b150c7e3056d Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Tue, 10 Dec 2024 20:12:05 -0800 Subject: [PATCH 5/6] OsDetachThread could be void --- src/coreclr/vm/ceemain.cpp | 5 ++--- src/coreclr/vm/ceemain.h | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/coreclr/vm/ceemain.cpp b/src/coreclr/vm/ceemain.cpp index 4d33044b674cbd..f476ea793c918b 100644 --- a/src/coreclr/vm/ceemain.cpp +++ b/src/coreclr/vm/ceemain.cpp @@ -1795,7 +1795,7 @@ static void OsAttachThread(void* thread) // thread - thread to detach // Return: // true if the thread was detached, false if there was no attached thread -bool OsDetachThread(void* thread) +void OsDetachThread(void* thread) { ASSERT(g_flsIndex != FLS_OUT_OF_INDEXES); void* threadFromCurrentFiber = FlsGetValue(g_flsIndex); @@ -1804,7 +1804,7 @@ bool OsDetachThread(void* thread) { // we've seen this thread, but not this fiber. It must be a "foreign" fiber that was // borrowing this thread. - return false; + return; } if (threadFromCurrentFiber != thread) @@ -1813,7 +1813,6 @@ bool OsDetachThread(void* thread) } FlsSetValue(g_flsIndex, NULL); - return true; } void EnsureTlsDestructionMonitor() diff --git a/src/coreclr/vm/ceemain.h b/src/coreclr/vm/ceemain.h index 69ba92d692d6d2..120082d30572ca 100644 --- a/src/coreclr/vm/ceemain.h +++ b/src/coreclr/vm/ceemain.h @@ -48,7 +48,7 @@ void ThreadDetaching(); void EnsureTlsDestructionMonitor(); #ifdef TARGET_WINDOWS void InitFlsSlot(); -bool OsDetachThread(void* thread); +void OsDetachThread(void* thread); #endif void SetLatchedExitCode (INT32 code); From a4eafe2590a36b03f215bd7f3b4c64c57824dbc8 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Tue, 10 Dec 2024 21:24:36 -0800 Subject: [PATCH 6/6] Update src/coreclr/vm/ceemain.cpp --- src/coreclr/vm/ceemain.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/coreclr/vm/ceemain.cpp b/src/coreclr/vm/ceemain.cpp index f476ea793c918b..eba08c952b21dc 100644 --- a/src/coreclr/vm/ceemain.cpp +++ b/src/coreclr/vm/ceemain.cpp @@ -1765,7 +1765,6 @@ void InitFlsSlot() g_flsIndex = FlsAlloc(FiberDetachCallback); if (g_flsIndex == FLS_OUT_OF_INDEXES) { - _ASSERTE(!"Initialization of an FLS slot failed."); COMPlusThrowWin32(); } }