From 190ddf5937253bc5554b6630f27a31f0910104df Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Thu, 25 Aug 2022 23:10:57 -0700 Subject: [PATCH 1/2] do not do shutdown for the main thread --- .../nativeaot/Runtime/windows/PalRedhawkMinWin.cpp | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp b/src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp index 32c072d05d06a2..691148656b0dd5 100644 --- a/src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp +++ b/src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp @@ -38,6 +38,8 @@ uint32_t PalEventWrite(REGHANDLE arg1, const EVENT_DESCRIPTOR * arg2, uint32_t a // Index for the fiber local storage of the attached thread pointer static uint32_t g_flsIndex = FLS_OUT_OF_INDEXES; +static uint32_t g_mainThreadId = 0; + // 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 @@ -50,7 +52,13 @@ void __stdcall FiberDetachCallback(void* lpFlsData) if (lpFlsData != NULL) { // The current fiber is the home fiber of a thread, so the thread is shutting down - RuntimeThreadShutdown(lpFlsData); + // Do not do shutdown for the main thread though. + // other threads are terminated before the main one and could leave TLS locked, + // so we will likely deadlock + if (GetCurrentThreadId() != g_mainThreadId) + { + RuntimeThreadShutdown(lpFlsData); + } } } @@ -137,6 +145,8 @@ void InitializeCurrentProcessCpuCount() // initialization and false on failure. REDHAWK_PALEXPORT bool REDHAWK_PALAPI PalInit() { + g_mainThreadId = GetCurrentThreadId(); + // 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); From 3fc0e68d9d2f34856ae7291b8e7587f273b49493 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Fri, 26 Aug 2022 16:55:33 -0700 Subject: [PATCH 2/2] specialcase the thread that called RhpShutdown instead --- src/coreclr/nativeaot/Runtime/startup.cpp | 18 +++++++++++++----- .../Runtime/windows/PalRedhawkMinWin.cpp | 12 +----------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/src/coreclr/nativeaot/Runtime/startup.cpp b/src/coreclr/nativeaot/Runtime/startup.cpp index 0cfa7efe36003f..62a0f3ba9221cb 100644 --- a/src/coreclr/nativeaot/Runtime/startup.cpp +++ b/src/coreclr/nativeaot/Runtime/startup.cpp @@ -401,7 +401,7 @@ static void UninitDLL() #endif // PROFILE_STARTUP } -volatile bool g_processShutdownHasStarted = false; +volatile Thread* g_threadPerformingShutdown = NULL; static void DllThreadDetach() { @@ -413,7 +413,7 @@ static void DllThreadDetach() { // Once shutdown starts, RuntimeThreadShutdown callbacks are ignored, implying that // it is no longer guaranteed that exiting threads will be detached. - if (!g_processShutdownHasStarted) + if (g_threadPerformingShutdown != NULL) { ASSERT_UNCONDITIONALLY("Detaching thread whose home fiber has not been detached"); RhFailFast(); @@ -439,9 +439,17 @@ void RuntimeThreadShutdown(void* thread) } #else ASSERT((Thread*)thread == ThreadStore::GetCurrentThread()); + + // Do not do shutdown for the thread that performs the shutdown. + // other threads could be terminated before it and could leave TLS locked + if ((Thread*)thread == g_threadPerformingShutdown) + { + return; + } + #endif - ThreadStore::DetachCurrentThread(g_processShutdownHasStarted); + ThreadStore::DetachCurrentThread(g_threadPerformingShutdown != NULL); } extern "C" bool RhInitialize() @@ -474,11 +482,11 @@ COOP_PINVOKE_HELPER(void, RhpEnableConservativeStackReporting, ()) COOP_PINVOKE_HELPER(void, RhpShutdown, ()) { // Indicate that runtime shutdown is complete and that the caller is about to start shutting down the entire process. - g_processShutdownHasStarted = true; + g_threadPerformingShutdown = ThreadStore::RawGetCurrentThread(); } #ifdef _WIN32 -EXTERN_C UInt32_BOOL WINAPI RtuDllMain(HANDLE hPalInstance, uint32_t dwReason, void* /*pvReserved*/) +EXTERN_C UInt32_BOOL WINAPI RtuDllMain(HANDLE hPalInstance, uint32_t dwReason, void* pvReserved) { switch (dwReason) { diff --git a/src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp b/src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp index 691148656b0dd5..32c072d05d06a2 100644 --- a/src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp +++ b/src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp @@ -38,8 +38,6 @@ uint32_t PalEventWrite(REGHANDLE arg1, const EVENT_DESCRIPTOR * arg2, uint32_t a // Index for the fiber local storage of the attached thread pointer static uint32_t g_flsIndex = FLS_OUT_OF_INDEXES; -static uint32_t g_mainThreadId = 0; - // 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 @@ -52,13 +50,7 @@ void __stdcall FiberDetachCallback(void* lpFlsData) if (lpFlsData != NULL) { // The current fiber is the home fiber of a thread, so the thread is shutting down - // Do not do shutdown for the main thread though. - // other threads are terminated before the main one and could leave TLS locked, - // so we will likely deadlock - if (GetCurrentThreadId() != g_mainThreadId) - { - RuntimeThreadShutdown(lpFlsData); - } + RuntimeThreadShutdown(lpFlsData); } } @@ -145,8 +137,6 @@ void InitializeCurrentProcessCpuCount() // initialization and false on failure. REDHAWK_PALEXPORT bool REDHAWK_PALAPI PalInit() { - g_mainThreadId = GetCurrentThreadId(); - // 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);