From faf3bf723320c046f08dcfbc9ea9d1bfbcea6f4a Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Wed, 5 Mar 2025 19:10:53 -0800 Subject: [PATCH 1/8] simplify detaching --- src/coreclr/nativeaot/Runtime/PalRedhawk.h | 1 - src/coreclr/nativeaot/Runtime/thread.cpp | 3 ++ src/coreclr/nativeaot/Runtime/threadstore.cpp | 17 +++++---- .../nativeaot/Runtime/unix/PalRedhawkUnix.cpp | 11 ------ .../Runtime/windows/PalRedhawkMinWin.cpp | 36 ++----------------- 5 files changed, 16 insertions(+), 52 deletions(-) diff --git a/src/coreclr/nativeaot/Runtime/PalRedhawk.h b/src/coreclr/nativeaot/Runtime/PalRedhawk.h index 6b93fc11a323e7..5b71c6c49132ef 100644 --- a/src/coreclr/nativeaot/Runtime/PalRedhawk.h +++ b/src/coreclr/nativeaot/Runtime/PalRedhawk.h @@ -321,7 +321,6 @@ REDHAWK_PALIMPORT uint32_t REDHAWK_PALAPI PalCompatibleWaitAny(UInt32_BOOL alert REDHAWK_PALIMPORT HANDLE PalCreateLowMemoryResourceNotification(); REDHAWK_PALIMPORT void REDHAWK_PALAPI PalAttachThread(void* thread); -REDHAWK_PALIMPORT bool REDHAWK_PALAPI PalDetachThread(void* thread); REDHAWK_PALIMPORT uint64_t PalGetCurrentOSThreadId(); diff --git a/src/coreclr/nativeaot/Runtime/thread.cpp b/src/coreclr/nativeaot/Runtime/thread.cpp index 77b00b10541ad7..a1a774a63ec691 100644 --- a/src/coreclr/nativeaot/Runtime/thread.cpp +++ b/src/coreclr/nativeaot/Runtime/thread.cpp @@ -1096,7 +1096,10 @@ bool Thread::IsDetached() void Thread::SetDetached() { ASSERT(!IsStateSet(TSF_Detached)); + ASSERT(IsStateSet(TSF_Attached)); + SetState(TSF_Detached); + ClearState(TSF_Attached); } bool Thread::IsActivationPending() diff --git a/src/coreclr/nativeaot/Runtime/threadstore.cpp b/src/coreclr/nativeaot/Runtime/threadstore.cpp index 6e32929ddb6b55..d03fc2393fe836 100644 --- a/src/coreclr/nativeaot/Runtime/threadstore.cpp +++ b/src/coreclr/nativeaot/Runtime/threadstore.cpp @@ -110,9 +110,16 @@ void ThreadStore::AttachCurrentThread(bool fAcquireThreadStoreLock) // we want to avoid at construction time because the loader lock is held then. Thread * pAttachingThread = RawGetCurrentThread(); - // The thread was already initialized, so it is already attached + if (pAttachingThread->IsDetached()) + { + ASSERT_UNCONDITIONALLY("The thread has been detached already"); + RhFailFast(); + } + + // The thread was already initialized, so it is already attached. if (pAttachingThread->IsInitialized()) { + ASSERT((pAttachingThread->m_ThreadStateFlags & Thread::TSF_Attached) != 0); return; } @@ -156,12 +163,8 @@ void ThreadStore::DetachCurrentThread() return; } - // Unregister from OS notifications - // This can return false if a thread did not register for OS notification. - if (!PalDetachThread(pDetachingThread)) - { - return; - } + // detach callback should not call us twice + ASSERT(!pDetachingThread->IsDetached()); // Run pre-mortem callbacks while we still can run managed code and not holding locks. // NOTE: background GC threads are attached/suspendable threads, but should not run ordinary diff --git a/src/coreclr/nativeaot/Runtime/unix/PalRedhawkUnix.cpp b/src/coreclr/nativeaot/Runtime/unix/PalRedhawkUnix.cpp index 24172b9c3707e5..a928e7018da25f 100644 --- a/src/coreclr/nativeaot/Runtime/unix/PalRedhawkUnix.cpp +++ b/src/coreclr/nativeaot/Runtime/unix/PalRedhawkUnix.cpp @@ -517,17 +517,6 @@ extern "C" void PalAttachThread(void* thread) UnmaskActivationSignal(); } -// Detach thread from OS notifications. -// Parameters: -// thread - thread to detach -// Return: -// true if the thread was detached, false if there was no attached thread -extern "C" bool PalDetachThread(void* thread) -{ - UNREFERENCED_PARAMETER(thread); - return true; -} - #if !defined(USE_PORTABLE_HELPERS) && !defined(FEATURE_RX_THUNKS) REDHAWK_PALEXPORT UInt32_BOOL REDHAWK_PALAPI PalAllocateThunksFromTemplate(HANDLE hTemplateModule, uint32_t templateRva, size_t templateSize, void** newThunksOut) diff --git a/src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp b/src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp index 844a1080e2def8..1a6e4e1ee90557 100644 --- a/src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp +++ b/src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp @@ -53,13 +53,11 @@ static uint32_t g_flsIndex = FLS_OUT_OF_INDEXES; void __stdcall FiberDetachCallback(void* lpFlsData) { ASSERT(g_flsIndex != FLS_OUT_OF_INDEXES); + ASSERT(g_flsIndex != NULL); 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); - } + // The current fiber is the home fiber of a thread, so the thread is shutting down + RuntimeThreadShutdown(lpFlsData); } static HMODULE LoadKernel32dll() @@ -199,34 +197,6 @@ REDHAWK_PALEXPORT void REDHAWK_PALAPI PalAttachThread(void* thread) 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 -REDHAWK_PALEXPORT bool REDHAWK_PALAPI PalDetachThread(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) - { - ASSERT_UNCONDITIONALLY("Detaching a thread from the wrong fiber"); - RhFailFast(); - } - - FlsSetValue(g_flsIndex, NULL); - return true; -} - extern "C" uint64_t PalQueryPerformanceCounter() { return GCToOSInterface::QueryPerformanceCounter(); From 3cb8c9766cf46b9bb4e0098f2a714b64ac70af49 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Wed, 5 Mar 2025 19:20:50 -0800 Subject: [PATCH 2/8] minor cleanups --- src/coreclr/nativeaot/Runtime/amd64/AsmMacros.inc | 1 - src/coreclr/nativeaot/Runtime/arm64/AsmMacros.h | 2 -- src/coreclr/nativeaot/Runtime/i386/AsmMacros.inc | 1 - src/coreclr/nativeaot/Runtime/thread.h | 4 +++- 4 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/coreclr/nativeaot/Runtime/amd64/AsmMacros.inc b/src/coreclr/nativeaot/Runtime/amd64/AsmMacros.inc index 4a3437f2f0618b..5d5cde13bced18 100644 --- a/src/coreclr/nativeaot/Runtime/amd64/AsmMacros.inc +++ b/src/coreclr/nativeaot/Runtime/amd64/AsmMacros.inc @@ -329,7 +329,6 @@ TAILJMP_RAX TEXTEQU ;; ;; CONSTANTS -- INTEGER ;; -TSF_Attached equ 01h TSF_SuppressGcStress equ 08h TSF_DoNotTriggerGc equ 10h diff --git a/src/coreclr/nativeaot/Runtime/arm64/AsmMacros.h b/src/coreclr/nativeaot/Runtime/arm64/AsmMacros.h index 90e1b5d7779947..89393563f51473 100644 --- a/src/coreclr/nativeaot/Runtime/arm64/AsmMacros.h +++ b/src/coreclr/nativeaot/Runtime/arm64/AsmMacros.h @@ -9,13 +9,11 @@ ;; ;; CONSTANTS -- INTEGER ;; -TSF_Attached equ 0x01 TSF_SuppressGcStress equ 0x08 TSF_DoNotTriggerGc equ 0x10 TSF_SuppressGcStress__OR__TSF_DoNotTriggerGC equ 0x18 ;; Bit position for the flags above, to be used with tbz/tbnz instructions -TSF_Attached_Bit equ 0 TSF_SuppressGcStress_Bit equ 3 TSF_DoNotTriggerGc_Bit equ 4 diff --git a/src/coreclr/nativeaot/Runtime/i386/AsmMacros.inc b/src/coreclr/nativeaot/Runtime/i386/AsmMacros.inc index 8ee2e79f44fde0..289d9ef15ac28d 100644 --- a/src/coreclr/nativeaot/Runtime/i386/AsmMacros.inc +++ b/src/coreclr/nativeaot/Runtime/i386/AsmMacros.inc @@ -110,7 +110,6 @@ endm ;; ;; CONSTANTS -- INTEGER ;; -TSF_Attached equ 01h TSF_SuppressGcStress equ 08h TSF_DoNotTriggerGc equ 10h diff --git a/src/coreclr/nativeaot/Runtime/thread.h b/src/coreclr/nativeaot/Runtime/thread.h index b4a2b0b5a62465..a431cd1434314b 100644 --- a/src/coreclr/nativeaot/Runtime/thread.h +++ b/src/coreclr/nativeaot/Runtime/thread.h @@ -189,7 +189,9 @@ class Thread : private RuntimeThreadLocals { TSF_Unknown = 0x00000000, // Threads are created in this state TSF_Attached = 0x00000001, // Thread was inited by first U->M transition on this thread - TSF_Detached = 0x00000002, // Thread was detached by DllMain + // ...Prior to setting this bit the state is TSF_Unknown. + TSF_Detached = 0x00000002, // Thread was detached and no longer can run managed code. + // ...TSF_Attached is cleared when TSF_Detached is set. TSF_SuppressGcStress = 0x00000008, // Do not allow gc stress on this thread, used in DllMain // ...and on the Finalizer thread TSF_DoNotTriggerGc = 0x00000010, // Do not allow hijacking of this thread, also intended to From 8bd7fde413b2471aa84a8f324f9ca7cf1fd5f690 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Wed, 5 Mar 2025 21:35:26 -0800 Subject: [PATCH 3/8] init COM + FLS in finalizer thread. --- .../nativeaot/Runtime/FinalizerHelpers.cpp | 19 +++++- src/coreclr/nativeaot/Runtime/GCHelpers.cpp | 16 ++++- src/coreclr/nativeaot/Runtime/PalRedhawk.h | 1 + .../Runtime/windows/PalRedhawkMinWin.cpp | 64 +++++++++++-------- 4 files changed, 67 insertions(+), 33 deletions(-) diff --git a/src/coreclr/nativeaot/Runtime/FinalizerHelpers.cpp b/src/coreclr/nativeaot/Runtime/FinalizerHelpers.cpp index 7f49f0830c241d..f2fb76441129e3 100644 --- a/src/coreclr/nativeaot/Runtime/FinalizerHelpers.cpp +++ b/src/coreclr/nativeaot/Runtime/FinalizerHelpers.cpp @@ -31,12 +31,19 @@ static HANDLE g_lowMemoryNotification = NULL; EXTERN_C void QCALLTYPE ProcessFinalizers(); -// Unmanaged front-end to the finalizer thread. We require this because at the point the GC creates the -// finalizer thread we can't run managed code. Instead this method waits +// Unmanaged front-end to the finalizer thread. We require this because at the point when this thread is +// created we can't run managed code. Instead this method waits // for the first finalization request (by which time everything must be up and running) and kicks off the // managed portion of the thread at that point uint32_t WINAPI FinalizerStart(void* pContext) { +#ifdef TARGET_WINDOWS + PalInitComAndFlsSlot(); + // handshake with EE initialization, as now we can attach Thread objects to native threads. + UInt32_BOOL res = PalSetEvent(g_FinalizerDoneEvent.GetOSEvent()); + ASSERT(res); +#endif // DEBUG + HANDLE hFinalizerEvent = (HANDLE)pContext; PalSetCurrentThreadName(".NET Finalizer"); @@ -86,6 +93,14 @@ bool RhInitializeFinalization() return true; } +#ifdef TARGET_WINDOWS +void RhWaitForFinalizerThreadStart() +{ + g_FinalizerDoneEvent.Wait(INFINITE,FALSE); + g_FinalizerDoneEvent.Reset(); +} +#endif + void RhEnableFinalization() { g_FinalizerEvent.Set(); diff --git a/src/coreclr/nativeaot/Runtime/GCHelpers.cpp b/src/coreclr/nativeaot/Runtime/GCHelpers.cpp index c0801faed16620..45d494bca09476 100644 --- a/src/coreclr/nativeaot/Runtime/GCHelpers.cpp +++ b/src/coreclr/nativeaot/Runtime/GCHelpers.cpp @@ -43,11 +43,18 @@ GPTR_DECL(MethodTable, g_pFreeObjectEEType); GPTR_IMPL(Thread, g_pFinalizerThread); bool RhInitializeFinalization(); +#ifdef TARGET_WINDOWS +void RhWaitForFinalizerThreadStart(); +#endif // Perform any runtime-startup initialization needed by the GC, HandleTable or environmental code in gcenv.ee. // Returns true on success or false if a subsystem failed to initialize. bool InitializeGC() { + // Give some headstart to the finalizer thread by launching it early. + if (!RhInitializeFinalization()) + return false; + // Initialize the special MethodTable used to mark free list entries in the GC heap. g_FreeObjectEEType.InitializeAsGcFreeType(); g_pFreeObjectEEType = &g_FreeObjectEEType; @@ -78,13 +85,16 @@ bool InitializeGC() if (FAILED(hr)) return false; - if (!RhInitializeFinalization()) - return false; - // Initialize HandleTable. if (!GCHandleUtilities::GetGCHandleManager()->Initialize()) return false; +#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. + RhWaitForFinalizerThreadStart(); +#endif return true; } diff --git a/src/coreclr/nativeaot/Runtime/PalRedhawk.h b/src/coreclr/nativeaot/Runtime/PalRedhawk.h index 5b71c6c49132ef..77fd4f71cb3a1b 100644 --- a/src/coreclr/nativeaot/Runtime/PalRedhawk.h +++ b/src/coreclr/nativeaot/Runtime/PalRedhawk.h @@ -295,6 +295,7 @@ typedef uint32_t (__stdcall *BackgroundCallback)(_In_opt_ void* pCallbackContext REDHAWK_PALIMPORT bool REDHAWK_PALAPI PalSetCurrentThreadName(const char* name); #ifdef TARGET_WINDOWS REDHAWK_PALIMPORT bool REDHAWK_PALAPI PalSetCurrentThreadNameW(const WCHAR* name); +REDHAWK_PALIMPORT void REDHAWK_PALAPI PalInitComAndFlsSlot(); #endif REDHAWK_PALIMPORT bool REDHAWK_PALAPI PalStartBackgroundGCThread(_In_ BackgroundCallback callback, _In_opt_ void* pCallbackContext); REDHAWK_PALIMPORT bool REDHAWK_PALAPI PalStartFinalizerThread(_In_ BackgroundCallback callback, _In_opt_ void* pCallbackContext); diff --git a/src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp b/src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp index 1a6e4e1ee90557..82e10b7cfcdf80 100644 --- a/src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp +++ b/src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp @@ -60,6 +60,42 @@ void __stdcall FiberDetachCallback(void* lpFlsData) RuntimeThreadShutdown(lpFlsData); } +REDHAWK_PALEXPORT void REDHAWK_PALAPI PalInitComAndFlsSlot() +{ + // Making finalizer thread MTA early ensures that COM is initialized before we initialize our thread + // termination callback. + CoInitializeEx(NULL, COINIT_MULTITHREADED); + + // 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) + { + ASSERT_UNCONDITIONALLY("Could not allocate an FLS slot."); + RhFailFast(); + } +} + +// 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 +REDHAWK_PALEXPORT void REDHAWK_PALAPI PalAttachThread(void* thread) +{ + void* threadFromCurrentFiber = FlsGetValue(g_flsIndex); + + if (threadFromCurrentFiber != NULL) + { + ASSERT_UNCONDITIONALLY("Multiple threads encountered from a single fiber"); + RhFailFast(); + } + + // 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); +} + static HMODULE LoadKernel32dll() { return LoadLibraryExW(L"kernel32", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32); @@ -157,14 +193,6 @@ void InitializeCurrentProcessCpuCount() // initialization and false on failure. REDHAWK_PALEXPORT bool REDHAWK_PALAPI PalInit() { - // 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; - } - GCConfig::Initialize(); if (!GCToOSInterface::Initialize()) @@ -177,26 +205,6 @@ REDHAWK_PALEXPORT bool REDHAWK_PALAPI PalInit() 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 -REDHAWK_PALEXPORT void REDHAWK_PALAPI PalAttachThread(void* thread) -{ - void* threadFromCurrentFiber = FlsGetValue(g_flsIndex); - - if (threadFromCurrentFiber != NULL) - { - ASSERT_UNCONDITIONALLY("Multiple threads encountered from a single fiber"); - RhFailFast(); - } - - // 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); -} - extern "C" uint64_t PalQueryPerformanceCounter() { return GCToOSInterface::QueryPerformanceCounter(); From 2618c732b345f0c79575f4adf8cb4c0b8f86014b Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Wed, 5 Mar 2025 22:52:11 -0800 Subject: [PATCH 4/8] remove managed finalizer+COM stuff --- .../src/System/Runtime/__Finalizer.cs | 4 ---- .../nativeaot/Runtime/FinalizerHelpers.cpp | 5 ---- .../src/System.Private.CoreLib.csproj | 1 - .../Runtime/InitializeFinalizerThread.cs | 20 ---------------- .../src/System/Runtime/RuntimeImports.cs | 3 --- .../System/Threading/Thread.NativeAot.Unix.cs | 4 ---- .../Threading/Thread.NativeAot.Windows.cs | 24 +++---------------- .../Runtime/InitializeFinalizerThread.cs | 16 ------------- .../Test.CoreLib/src/Test.CoreLib.csproj | 1 - 9 files changed, 3 insertions(+), 75 deletions(-) delete mode 100644 src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InitializeFinalizerThread.cs delete mode 100644 src/coreclr/nativeaot/Test.CoreLib/src/System/Runtime/InitializeFinalizerThread.cs diff --git a/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/__Finalizer.cs b/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/__Finalizer.cs index dce232600ff1ce..e0573bf5c1b830 100644 --- a/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/__Finalizer.cs +++ b/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/__Finalizer.cs @@ -20,10 +20,6 @@ internal static class __Finalizer [UnmanagedCallersOnly(EntryPoint = "ProcessFinalizers")] public static void ProcessFinalizers() { -#if INPLACE_RUNTIME - System.Runtime.FinalizerInitRunner.DoInitialize(); -#endif - while (true) { // Wait until there's some work to be done. If true is returned we should finalize objects, diff --git a/src/coreclr/nativeaot/Runtime/FinalizerHelpers.cpp b/src/coreclr/nativeaot/Runtime/FinalizerHelpers.cpp index f2fb76441129e3..480913fcec1da9 100644 --- a/src/coreclr/nativeaot/Runtime/FinalizerHelpers.cpp +++ b/src/coreclr/nativeaot/Runtime/FinalizerHelpers.cpp @@ -106,11 +106,6 @@ void RhEnableFinalization() g_FinalizerEvent.Set(); } -EXTERN_C void QCALLTYPE RhInitializeFinalizerThread() -{ - g_FinalizerEvent.Set(); -} - static int32_t g_fullGcCountSeenByFinalization; // Indicate that the current round of finalizations is complete. diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System.Private.CoreLib.csproj b/src/coreclr/nativeaot/System.Private.CoreLib/src/System.Private.CoreLib.csproj index 7bb3b0f58e70e5..71646b278f0f3c 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System.Private.CoreLib.csproj +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System.Private.CoreLib.csproj @@ -201,7 +201,6 @@ - diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InitializeFinalizerThread.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InitializeFinalizerThread.cs deleted file mode 100644 index d0021229b7522c..00000000000000 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InitializeFinalizerThread.cs +++ /dev/null @@ -1,20 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Threading; - -namespace System.Runtime -{ - internal static class FinalizerInitRunner - { - // Here, we are subscribing to a callback from the runtime. This callback is made from the finalizer - // thread before any objects are finalized. - [RuntimeExport("InitializeFinalizerThread")] - public static void DoInitialize() - { - // Make sure that the finalizer thread is CoInitialized before any objects are finalized. If this - // fails, it will throw an exception and that will go unhandled, triggering a FailFast. - Thread.InitializeComForFinalizerThread(); - } - } -} diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/RuntimeImports.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/RuntimeImports.cs index b27d8de76e17b6..4fe7ee2e706106 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/RuntimeImports.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/RuntimeImports.cs @@ -94,9 +94,6 @@ internal static void RhWaitForPendingFinalizers(bool allowReentrantWait) RhWaitForPendingFinalizers(allowReentrantWait ? 1 : 0); } - [LibraryImport(RuntimeLibrary)] - internal static partial void RhInitializeFinalizerThread(); - // Get maximum GC generation number. [MethodImplAttribute(MethodImplOptions.InternalCall)] [RuntimeImport(RuntimeLibrary, "RhGetMaxGcGeneration")] diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.Unix.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.Unix.cs index 3cf12153d1f6d6..f9e49db5323045 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.Unix.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.Unix.cs @@ -137,10 +137,6 @@ private static bool SetApartmentStateUnchecked(ApartmentState state, bool throwO partial void InitializeComOnNewThread(); - internal static void InitializeComForFinalizerThread() - { - } - public void DisableComObjectEagerCleanup() { } public void Interrupt() => WaitSubsystem.Interrupt(this); diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.Windows.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.Windows.cs index 15f8c25ae42eee..c579f4b3a00e32 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.Windows.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.Windows.cs @@ -24,8 +24,6 @@ public sealed partial class Thread private ApartmentState _initialApartmentState = ApartmentState.Unknown; - private static volatile bool s_comInitializedOnFinalizerThread; - partial void PlatformSpecificInitialize(); // Platform-specific initialization of foreign threads, i.e. threads not created by Thread.Start @@ -301,27 +299,11 @@ private void InitializeComOnNewThread() InitializeCom(_initialApartmentState); } - internal static void InitializeComForFinalizerThread() - { - InitializeCom(); - - // Prevent re-initialization of COM model on finalizer thread - t_comState |= ComState.Locked; - - s_comInitializedOnFinalizerThread = true; - } - private static void InitializeComForThreadPoolThread() { - // Initialized COM - take advantage of implicit MTA initialized by the finalizer thread - SpinWait sw = default(SpinWait); - while (!s_comInitializedOnFinalizerThread) - { - RuntimeImports.RhInitializeFinalizerThread(); - sw.SpinOnce(0); - } - - // Prevent re-initialization of COM model on threadpool threads + // Process-wide COM is initialized very early before any managed code can run. + // Assume it is done. + // Prevent re-initialization of COM model on threadpool threads from the default one. t_comState |= ComState.Locked; } diff --git a/src/coreclr/nativeaot/Test.CoreLib/src/System/Runtime/InitializeFinalizerThread.cs b/src/coreclr/nativeaot/Test.CoreLib/src/System/Runtime/InitializeFinalizerThread.cs deleted file mode 100644 index 08ca09dd867bbc..00000000000000 --- a/src/coreclr/nativeaot/Test.CoreLib/src/System/Runtime/InitializeFinalizerThread.cs +++ /dev/null @@ -1,16 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Threading; - -namespace System.Runtime -{ - internal static class FinalizerInitRunner - { - // Here, we are subscribing to a callback from the runtime. This callback is made from the finalizer - // thread before any objects are finalized. - public static void DoInitialize() - { - } - } -} diff --git a/src/coreclr/nativeaot/Test.CoreLib/src/Test.CoreLib.csproj b/src/coreclr/nativeaot/Test.CoreLib/src/Test.CoreLib.csproj index b838c2ed7c14b7..7395abd155da45 100644 --- a/src/coreclr/nativeaot/Test.CoreLib/src/Test.CoreLib.csproj +++ b/src/coreclr/nativeaot/Test.CoreLib/src/Test.CoreLib.csproj @@ -229,7 +229,6 @@ - From 7382fd64e96bc24256b64db6d03ef630c1082bf8 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Mon, 10 Mar 2025 19:59:15 -0700 Subject: [PATCH 5/8] Unify assert message with CoreCLR --- src/coreclr/nativeaot/Runtime/threadstore.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/nativeaot/Runtime/threadstore.cpp b/src/coreclr/nativeaot/Runtime/threadstore.cpp index d03fc2393fe836..dbd321ffcff4e6 100644 --- a/src/coreclr/nativeaot/Runtime/threadstore.cpp +++ b/src/coreclr/nativeaot/Runtime/threadstore.cpp @@ -112,7 +112,7 @@ void ThreadStore::AttachCurrentThread(bool fAcquireThreadStoreLock) if (pAttachingThread->IsDetached()) { - ASSERT_UNCONDITIONALLY("The thread has been detached already"); + ASSERT_UNCONDITIONALLY("Attempt to execute managed code after the .NET runtime thread state has been destroyed."); RhFailFast(); } From 88494801cdf4ddb496e78bf4d9165fe7f9273206 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Tue, 11 Mar 2025 11:15:07 -0700 Subject: [PATCH 6/8] Treat failure to allocate FLS slot similar to OOM. --- src/coreclr/nativeaot/Runtime/FinalizerHelpers.cpp | 9 +++++++-- src/coreclr/nativeaot/Runtime/GCHelpers.cpp | 5 +++-- src/coreclr/nativeaot/Runtime/PalRedhawk.h | 2 +- .../nativeaot/Runtime/windows/PalRedhawkMinWin.cpp | 10 ++++------ 4 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/coreclr/nativeaot/Runtime/FinalizerHelpers.cpp b/src/coreclr/nativeaot/Runtime/FinalizerHelpers.cpp index 480913fcec1da9..c90e54d22fcfcd 100644 --- a/src/coreclr/nativeaot/Runtime/FinalizerHelpers.cpp +++ b/src/coreclr/nativeaot/Runtime/FinalizerHelpers.cpp @@ -29,6 +29,10 @@ CLREventStatic g_FinalizerDoneEvent; static HANDLE g_lowMemoryNotification = NULL; +#ifdef TARGET_WINDOWS +static bool g_FlsInitSucceeded = false; +#endif + EXTERN_C void QCALLTYPE ProcessFinalizers(); // Unmanaged front-end to the finalizer thread. We require this because at the point when this thread is @@ -38,7 +42,7 @@ EXTERN_C void QCALLTYPE ProcessFinalizers(); uint32_t WINAPI FinalizerStart(void* pContext) { #ifdef TARGET_WINDOWS - PalInitComAndFlsSlot(); + g_FlsInitSucceeded = PalInitComAndFlsSlot(); // handshake with EE initialization, as now we can attach Thread objects to native threads. UInt32_BOOL res = PalSetEvent(g_FinalizerDoneEvent.GetOSEvent()); ASSERT(res); @@ -94,10 +98,11 @@ bool RhInitializeFinalization() } #ifdef TARGET_WINDOWS -void RhWaitForFinalizerThreadStart() +bool RhWaitForFinalizerThreadStart() { g_FinalizerDoneEvent.Wait(INFINITE,FALSE); g_FinalizerDoneEvent.Reset(); + return g_FlsInitSucceeded; } #endif diff --git a/src/coreclr/nativeaot/Runtime/GCHelpers.cpp b/src/coreclr/nativeaot/Runtime/GCHelpers.cpp index 45d494bca09476..91fd2db9260622 100644 --- a/src/coreclr/nativeaot/Runtime/GCHelpers.cpp +++ b/src/coreclr/nativeaot/Runtime/GCHelpers.cpp @@ -44,7 +44,7 @@ GPTR_IMPL(Thread, g_pFinalizerThread); bool RhInitializeFinalization(); #ifdef TARGET_WINDOWS -void RhWaitForFinalizerThreadStart(); +bool RhWaitForFinalizerThreadStart(); #endif // Perform any runtime-startup initialization needed by the GC, HandleTable or environmental code in gcenv.ee. @@ -93,7 +93,8 @@ bool InitializeGC() // 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. - RhWaitForFinalizerThreadStart(); + if (!RhWaitForFinalizerThreadStart()) + return false; #endif return true; } diff --git a/src/coreclr/nativeaot/Runtime/PalRedhawk.h b/src/coreclr/nativeaot/Runtime/PalRedhawk.h index 77fd4f71cb3a1b..adc2deaf208e5b 100644 --- a/src/coreclr/nativeaot/Runtime/PalRedhawk.h +++ b/src/coreclr/nativeaot/Runtime/PalRedhawk.h @@ -295,7 +295,7 @@ typedef uint32_t (__stdcall *BackgroundCallback)(_In_opt_ void* pCallbackContext REDHAWK_PALIMPORT bool REDHAWK_PALAPI PalSetCurrentThreadName(const char* name); #ifdef TARGET_WINDOWS REDHAWK_PALIMPORT bool REDHAWK_PALAPI PalSetCurrentThreadNameW(const WCHAR* name); -REDHAWK_PALIMPORT void REDHAWK_PALAPI PalInitComAndFlsSlot(); +REDHAWK_PALIMPORT bool REDHAWK_PALAPI PalInitComAndFlsSlot(); #endif REDHAWK_PALIMPORT bool REDHAWK_PALAPI PalStartBackgroundGCThread(_In_ BackgroundCallback callback, _In_opt_ void* pCallbackContext); REDHAWK_PALIMPORT bool REDHAWK_PALAPI PalStartFinalizerThread(_In_ BackgroundCallback callback, _In_opt_ void* pCallbackContext); diff --git a/src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp b/src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp index 82e10b7cfcdf80..5196a05e56e1ff 100644 --- a/src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp +++ b/src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp @@ -60,8 +60,10 @@ void __stdcall FiberDetachCallback(void* lpFlsData) RuntimeThreadShutdown(lpFlsData); } -REDHAWK_PALEXPORT void REDHAWK_PALAPI PalInitComAndFlsSlot() +REDHAWK_PALEXPORT bool REDHAWK_PALAPI PalInitComAndFlsSlot() { + ASSERT(g_flsIndex == FLS_OUT_OF_INDEXES); + // Making finalizer thread MTA early ensures that COM is initialized before we initialize our thread // termination callback. CoInitializeEx(NULL, COINIT_MULTITHREADED); @@ -69,11 +71,7 @@ REDHAWK_PALEXPORT void REDHAWK_PALAPI PalInitComAndFlsSlot() // 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) - { - ASSERT_UNCONDITIONALLY("Could not allocate an FLS slot."); - RhFailFast(); - } + return g_flsIndex != FLS_OUT_OF_INDEXES; } // Register the thread with OS to be notified when thread is about to be destroyed From 7119c3787be9b0842f3e40f48b63e551303a0f3f Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Tue, 11 Mar 2025 13:16:11 -0700 Subject: [PATCH 7/8] Exit from finalizer thread if FLS or COM failed to initialize. --- src/coreclr/nativeaot/Runtime/FinalizerHelpers.cpp | 11 ++++++++--- .../nativeaot/Runtime/windows/PalRedhawkMinWin.cpp | 4 ++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/coreclr/nativeaot/Runtime/FinalizerHelpers.cpp b/src/coreclr/nativeaot/Runtime/FinalizerHelpers.cpp index c90e54d22fcfcd..730edbc87a3a3b 100644 --- a/src/coreclr/nativeaot/Runtime/FinalizerHelpers.cpp +++ b/src/coreclr/nativeaot/Runtime/FinalizerHelpers.cpp @@ -30,7 +30,7 @@ CLREventStatic g_FinalizerDoneEvent; static HANDLE g_lowMemoryNotification = NULL; #ifdef TARGET_WINDOWS -static bool g_FlsInitSucceeded = false; +static bool g_ComAndFlsInitSucceeded = false; #endif EXTERN_C void QCALLTYPE ProcessFinalizers(); @@ -42,10 +42,15 @@ EXTERN_C void QCALLTYPE ProcessFinalizers(); uint32_t WINAPI FinalizerStart(void* pContext) { #ifdef TARGET_WINDOWS - g_FlsInitSucceeded = PalInitComAndFlsSlot(); + g_ComAndFlsInitSucceeded = PalInitComAndFlsSlot(); // handshake with EE initialization, as now we can attach Thread objects to native threads. UInt32_BOOL res = PalSetEvent(g_FinalizerDoneEvent.GetOSEvent()); ASSERT(res); + + // if FLS initialization failed do not attach the current thread and just exit instead. + // we are going to fail the runtime initialization. + if (!g_ComAndFlsInitSucceeded) + return 0; #endif // DEBUG HANDLE hFinalizerEvent = (HANDLE)pContext; @@ -102,7 +107,7 @@ bool RhWaitForFinalizerThreadStart() { g_FinalizerDoneEvent.Wait(INFINITE,FALSE); g_FinalizerDoneEvent.Reset(); - return g_FlsInitSucceeded; + return g_ComAndFlsInitSucceeded; } #endif diff --git a/src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp b/src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp index 5196a05e56e1ff..5445fb9dfeb039 100644 --- a/src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp +++ b/src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp @@ -66,12 +66,12 @@ REDHAWK_PALEXPORT bool REDHAWK_PALAPI PalInitComAndFlsSlot() // Making finalizer thread MTA early ensures that COM is initialized before we initialize our thread // termination callback. - CoInitializeEx(NULL, COINIT_MULTITHREADED); + HRESULT hr = CoInitializeEx(NULL, COINIT_MULTITHREADED); // 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); - return g_flsIndex != FLS_OUT_OF_INDEXES; + return SUCCEEDED(hr) && g_flsIndex != FLS_OUT_OF_INDEXES; } // Register the thread with OS to be notified when thread is about to be destroyed From 40d96d126bbbc2070cf7bfc3e558504b6d0ba893 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Tue, 11 Mar 2025 13:28:45 -0700 Subject: [PATCH 8/8] Apply suggestions from code review --- src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp b/src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp index 5445fb9dfeb039..0b8b0cb6400325 100644 --- a/src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp +++ b/src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp @@ -67,11 +67,13 @@ REDHAWK_PALEXPORT bool REDHAWK_PALAPI PalInitComAndFlsSlot() // Making finalizer thread MTA early ensures that COM is initialized before we initialize our thread // termination callback. HRESULT hr = CoInitializeEx(NULL, COINIT_MULTITHREADED); + if (FAILED(hr)) + return false; // 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); - return SUCCEEDED(hr) && g_flsIndex != FLS_OUT_OF_INDEXES; + return g_flsIndex != FLS_OUT_OF_INDEXES; } // Register the thread with OS to be notified when thread is about to be destroyed