From de03e43c425ea61ab301b2c3d5c3f4fdf954fb47 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/7] 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 c2b42491a387d..c03907e4eed66 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 3a1ab790f4753..39433a88cc185 100644 --- a/src/coreclr/vm/ceemain.cpp +++ b/src/coreclr/vm/ceemain.cpp @@ -1707,6 +1707,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; @@ -1720,36 +1844,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()); } } }; @@ -1763,6 +1858,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 1404a5a04237f..fec164ea3920b 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 8c60b2b5a7982..8617aa260f52c 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(); @@ -1065,6 +1074,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 6223b484e181235d1b7c6b522f761c55e4d885a3 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/7] 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 39433a88cc185..87c9a81fdcdfd 100644 --- a/src/coreclr/vm/ceemain.cpp +++ b/src/coreclr/vm/ceemain.cpp @@ -1787,8 +1787,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" @@ -1817,8 +1816,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 37a141e9833e11e0153911bc9351a0c3ea51547e 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/7] 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 8617aa260f52c..3a8b001d22d8e 100644 --- a/src/coreclr/vm/threads.cpp +++ b/src/coreclr/vm/threads.cpp @@ -1077,8 +1077,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 18e3acf364746bc0bd5bc2c686a1ae52b876d30a 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/7] 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 87c9a81fdcdfd..e69506c29382f 100644 --- a/src/coreclr/vm/ceemain.cpp +++ b/src/coreclr/vm/ceemain.cpp @@ -1764,17 +1764,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 fec164ea3920b..69ba92d692d6d 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 3a8b001d22d8e..c42dbc431ea97 100644 --- a/src/coreclr/vm/threads.cpp +++ b/src/coreclr/vm/threads.cpp @@ -1075,10 +1075,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 3dd3918fbb7dc8506563ab45ef0163ff50a97c93 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/7] 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 e69506c29382f..161470b914890 100644 --- a/src/coreclr/vm/ceemain.cpp +++ b/src/coreclr/vm/ceemain.cpp @@ -1801,7 +1801,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); @@ -1810,7 +1810,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) @@ -1819,7 +1819,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 69ba92d692d6d..120082d30572c 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 e58420ac07de1c22f5f094b86125d9c7e95b5d37 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Tue, 10 Dec 2024 21:24:36 -0800 Subject: [PATCH 6/7] 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 161470b914890..04106c5f4b65d 100644 --- a/src/coreclr/vm/ceemain.cpp +++ b/src/coreclr/vm/ceemain.cpp @@ -1771,7 +1771,6 @@ void InitFlsSlot() g_flsIndex = FlsAlloc(FiberDetachCallback); if (g_flsIndex == FLS_OUT_OF_INDEXES) { - _ASSERTE(!"Initialization of an FLS slot failed."); COMPlusThrowWin32(); } } From 5da4c1b46e881068ab49f2b1e0fd11ab708a9a60 Mon Sep 17 00:00:00 2001 From: Vladimir Sadov Date: Wed, 11 Dec 2024 13:43:26 -0800 Subject: [PATCH 7/7] Fix a comment --- src/coreclr/vm/ceemain.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/coreclr/vm/ceemain.cpp b/src/coreclr/vm/ceemain.cpp index 04106c5f4b65d..ae24f7cf23c11 100644 --- a/src/coreclr/vm/ceemain.cpp +++ b/src/coreclr/vm/ceemain.cpp @@ -1798,8 +1798,6 @@ static void OsAttachThread(void* thread) // 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 void OsDetachThread(void* thread) { ASSERT(g_flsIndex != FLS_OUT_OF_INDEXES);