[NativeAOT] Cleanup and rationalizing process/thread termination scen…

…ario (#80063)

* remove m_pTEB

* set g_threadPerformingShutdown in DllMain

* more thread detach cleanup

* rationalizing DetachCurrentThread

* OnProcessExit

* Remove g_SuspendEELock

* fixes

* comment tweak

* fix assert (thread could be detached before ending up performing shutdown)

* Apply suggestions from code review

Co-authored-by: Jan Kotas <jkotas@microsoft.com>

* remove unused RtuDllMain

Co-authored-by: Jan Kotas <jkotas@microsoft.com>

src/coreclr/nativeaot/Bootstrap/main.cpp

@@ -94,7 +94,6 @@ static char& __unbox_z = __stop___unbox;
 #endif // _MSC_VER
 
 extern "C" bool RhInitialize();
-extern "C" void RhpShutdown();
 extern "C" void RhSetRuntimeInitializationCallback(int (*fPtr)());
 
 extern "C" bool RhRegisterOSModule(void * pModule,
@@ -213,11 +212,7 @@ int main(int argc, char* argv[])
     if (initval != 0)
         return initval;
 
-    int retval = __managed__Main(argc, argv);
-
-    RhpShutdown();
-
-    return retval;
+    return __managed__Main(argc, argv);
 }
 #endif // !NATIVEAOT_DLL
 

src/coreclr/nativeaot/Runtime/Crst.h

@@ -8,6 +8,9 @@
 // functionality (in particular there is no rank violation checking).
 //
 
+#ifndef __Crst_h__
+#define __Crst_h__
+
 enum CrstType
 {
     CrstHandleTable,
@@ -20,7 +23,7 @@ enum CrstType
     CrstRestrictedCallouts,
     CrstObjectiveCMarshalCallouts,
     CrstGcStressControl,
-    CrstSuspendEE,
+    CrstThreadStore,
     CrstCastCache,
     CrstYieldProcessorNormalized,
 };
@@ -126,3 +129,5 @@ class CrstHolderWithState
         return m_pLock;
     }
 };
+
+#endif //__Crst_h__

src/coreclr/nativeaot/Runtime/FinalizerHelpers.cpp

@@ -29,14 +29,12 @@ GPTR_DECL(Thread, g_pFinalizerThread);
 CLREventStatic g_FinalizerEvent;
 CLREventStatic g_FinalizerDoneEvent;
 
-// Finalizer method implemented by redhawkm.
 extern "C" void __cdecl ProcessFinalizers();
 
 // Unmanaged front-end to the finalizer thread. We require this because at the point the GC creates the
-// finalizer thread we're still executing the DllMain for RedhawkU. At that point we can't run managed code
-// successfully (in particular module initialization code has not run for RedhawkM). Instead this method waits
+// finalizer thread 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.
+// managed portion of the thread at that point
 uint32_t WINAPI FinalizerStart(void* pContext)
 {
     HANDLE hFinalizerEvent = (HANDLE)pContext;
@@ -62,8 +60,7 @@ uint32_t WINAPI FinalizerStart(void* pContext)
     UInt32_BOOL fResult = PalSetEvent(hFinalizerEvent);
     ASSERT(fResult);
 
-    // Run the managed portion of the finalizer. Until we implement (non-process) shutdown this call will
-    // never return.
+    // Run the managed portion of the finalizer. This call will never return.
 
     ProcessFinalizers();
 

src/coreclr/nativeaot/Runtime/gcrhenv.cpp

@@ -144,7 +144,6 @@ uint32_t EtwCallback(uint32_t IsEnabled, RH_ETW_CONTEXT * pContext)
 // success or false if a subsystem failed to initialize.
 
 #ifndef DACCESS_COMPILE
-CrstStatic g_SuspendEELock;
 #ifdef _MSC_VER
 #pragma warning(disable:4815) // zero-sized array in stack object will have no elements
 #endif // _MSC_VER
@@ -169,9 +168,6 @@ bool RedhawkGCInterface::InitializeSubsystems()
     g_FreeObjectEEType.InitializeAsGcFreeType();
     g_pFreeObjectEEType = &g_FreeObjectEEType;
 
-    if (!g_SuspendEELock.InitNoThrow(CrstSuspendEE))
-        return false;
-
 #ifdef FEATURE_SVR_GC
     g_heap_type = (g_pRhConfig->GetgcServer() && PalGetProcessCpuCount() > 1) ? GC_HEAP_SVR : GC_HEAP_WKS;
 #else
@@ -642,10 +638,8 @@ void GCToEEInterface::SuspendEE(SUSPEND_REASON reason)
 
     FireEtwGCSuspendEEBegin_V1(Info.SuspendEE.Reason, Info.SuspendEE.GcCount, GetClrInstanceId());
 
-    g_SuspendEELock.Enter();
-
+    GetThreadStore()->LockThreadStore();
     GCHeapUtilities::GetGCHeap()->SetGCInProgress(TRUE);
-
     GetThreadStore()->SuspendAllThreads(true);
 
     FireEtwGCSuspendEEEnd_V1(GetClrInstanceId());
@@ -669,8 +663,7 @@ void GCToEEInterface::RestartEE(bool /*bFinishedGC*/)
 
     GetThreadStore()->ResumeAllThreads(true);
     GCHeapUtilities::GetGCHeap()->SetGCInProgress(FALSE);
-
-    g_SuspendEELock.Leave();
+    GetThreadStore()->UnlockThreadStore();
 
     FireEtwGCRestartEEEnd_V1(GetClrInstanceId());
 }

src/coreclr/nativeaot/Runtime/startup.cpp

@@ -443,25 +443,25 @@ static void UninitDLL()
 #endif // PROFILE_STARTUP
 }
 
-volatile Thread* g_threadPerformingShutdown = NULL;
-
-static void DllThreadDetach()
+#ifdef _WIN32
+// This is set to the thread that initiates and performs the shutdown and may run
+// after other threads are rudely terminated. So far this is a Windows-specific concern.
+// 
+// On POSIX OSes a process typically lives as long as any of its threads are alive or until
+// the process is terminated via `exit()` or a signal. Thus there is no such distinction
+// between threads.
+Thread* g_threadPerformingShutdown = NULL;
+
+static void __cdecl OnProcessExit()
 {
-    // BEWARE: loader lock is held here!
-
-    // Should have already received a call to FiberDetach for this thread's "home" fiber.
-    Thread* pCurrentThread = ThreadStore::GetCurrentThreadIfAvailable();
-    if (pCurrentThread != NULL && !pCurrentThread->IsDetached())
-    {
-        // Once shutdown starts, RuntimeThreadShutdown callbacks are ignored, implying that
-        // it is no longer guaranteed that exiting threads will be detached.
-        if (g_threadPerformingShutdown != NULL)
-        {
-            ASSERT_UNCONDITIONALLY("Detaching thread whose home fiber has not been detached");
-            RhFailFast();
-        }
-    }
+    // The process is exiting and the current thread is performing the shutdown.
+    // When this thread exits some threads may be already rudely terminated.
+    // It would not be a good idea for this thread to wait on any locks
+    // or run managed code at shutdown, so we will not try detaching it.
+    Thread* currentThread = ThreadStore::RawGetCurrentThread();
+    g_threadPerformingShutdown = currentThread;
 }
+#endif
 
 void RuntimeThreadShutdown(void* thread)
 {
@@ -470,35 +470,38 @@ void RuntimeThreadShutdown(void* thread)
     // that is made for the single thread that runs the final stages of orderly process
     // shutdown (i.e., the thread that delivers the DLL_PROCESS_DETACH notifications when the
     // process is being torn down via an ExitProcess call).
+    // In such case we do not detach.
 
-    UNREFERENCED_PARAMETER(thread);
+#ifdef _WIN32
+    ASSERT((Thread*)thread == ThreadStore::GetCurrentThread());
 
-#ifdef TARGET_UNIX
-    // Some Linux toolset versions call thread-local destructors during shutdown on a wrong thread.
-    if ((Thread*)thread != ThreadStore::GetCurrentThread())
+    // Do not try detaching the thread that performs the shutdown.
+    if (g_threadPerformingShutdown == thread)
     {
+        // At this point other threads could be terminated rudely while leaving runtime
+        // in inconsistent state, so we would be risking blocking the process from exiting.
         return;
     }
 #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)
+    // Some Linux toolset versions call thread-local destructors during shutdown on a wrong thread.
+    if ((Thread*)thread != ThreadStore::GetCurrentThread())
     {
         return;
     }
-
 #endif
 
-    ThreadStore::DetachCurrentThread(g_threadPerformingShutdown != NULL);
+    ThreadStore::DetachCurrentThread();
 }
 
 extern "C" bool RhInitialize()
 {
     if (!PalInit())
         return false;
 
+#ifdef _WIN32
+    atexit(&OnProcessExit);
+#endif
+
     if (!InitDLL(PalGetModuleHandleFromPointer((void*)&RhInitialize)))
         return false;
 
@@ -508,47 +511,4 @@ extern "C" bool RhInitialize()
     return true;
 }
 
-//
-// Currently called only from a managed executable once Main returns, this routine does whatever is needed to
-// cleanup managed state before exiting. There's not a lot here at the moment since we're always about to let
-// the OS tear the process down anyway.
-//
-// @TODO: Eventually we'll probably have a hosting API and explicit shutdown request. When that happens we'll
-// something more sophisticated here since we won't be able to rely on the OS cleaning up after us.
-//
-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_threadPerformingShutdown = ThreadStore::RawGetCurrentThread();
-}
-
-#ifdef _WIN32
-EXTERN_C UInt32_BOOL WINAPI RtuDllMain(HANDLE hPalInstance, uint32_t dwReason, void* pvReserved)
-{
-    switch (dwReason)
-    {
-    case DLL_PROCESS_ATTACH:
-    {
-        STARTUP_TIMELINE_EVENT(PROCESS_ATTACH_BEGIN);
-
-        if (!InitDLL(hPalInstance))
-            return FALSE;
-
-        STARTUP_TIMELINE_EVENT(PROCESS_ATTACH_COMPLETE);
-    }
-    break;
-
-    case DLL_PROCESS_DETACH:
-        UninitDLL();
-        break;
-
-    case DLL_THREAD_DETACH:
-        DllThreadDetach();
-        break;
-    }
-
-    return TRUE;
-}
-#endif // _WIN32
-
 #endif // !DACCESS_COMPILE

src/coreclr/nativeaot/Runtime/thread.cpp

@@ -278,8 +278,6 @@ void Thread::Construct()
     if (!PalGetMaximumStackBounds(&m_pStackLow, &m_pStackHigh))
         RhFailFast();
 
-    m_pTEB = PalNtCurrentTeb();
-
 #ifdef STRESS_LOG
     if (StressLog::StressLogOn(~0u, 0))
         m_pThreadStressLog = StressLog::CreateThreadStressLog(this);
@@ -341,12 +339,8 @@ bool Thread::IsCurrentThread()
 
 void Thread::Detach()
 {
-    // Thread::Destroy is called when the thread's "home" fiber dies.  We mark the thread as "detached" here
-    // so that we can validate, in our DLL_THREAD_DETACH handler, that the thread was already destroyed at that
-    // point.
-    SetDetached();
-
     RedhawkGCInterface::ReleaseAllocContext(GetAllocContext());
+    SetDetached();
 }
 
 void Thread::Destroy()
@@ -1099,20 +1093,6 @@ void Thread::ValidateExInfoStack()
 #endif // !DACCESS_COMPILE
 }
 
-
-
-// Retrieve the start of the TLS storage block allocated for the given thread for a specific module identified
-// by the TLS slot index allocated to that module and the offset into the OS allocated block at which
-// Redhawk-specific data is stored.
-PTR_UInt8 Thread::GetThreadLocalStorage(uint32_t uTlsIndex, uint32_t uTlsStartOffset)
-{
-#if 0
-    return (*(uint8_t***)(m_pTEB + OFFSETOF__TEB__ThreadLocalStoragePointer))[uTlsIndex] + uTlsStartOffset;
-#else
-    return (*dac_cast<PTR_PTR_PTR_UInt8>(dac_cast<TADDR>(m_pTEB) + OFFSETOF__TEB__ThreadLocalStoragePointer))[uTlsIndex] + uTlsStartOffset;
-#endif
-}
-
 #ifndef DACCESS_COMPILE
 
 #ifndef TARGET_UNIX

src/coreclr/nativeaot/Runtime/thread.h

@@ -37,19 +37,11 @@ class Thread;
 // can be retrieved via GetInterruptedContext()
 #define INTERRUPTED_THREAD_MARKER ((PInvokeTransitionFrame*)(ptrdiff_t)-2)
 
-enum SyncRequestResult
-{
-    TryAgain,
-    SuccessUnmanaged,
-    SuccessManaged,
-};
-
 typedef DPTR(PAL_LIMITED_CONTEXT) PTR_PAL_LIMITED_CONTEXT;
 
 struct ExInfo;
 typedef DPTR(ExInfo) PTR_ExInfo;
 
-
 // Also defined in ExceptionHandling.cs, layouts must match.
 // When adding new fields to this struct, ensure they get properly initialized in the exception handling
 // assembly stubs
@@ -94,7 +86,6 @@ struct ThreadBuffer
     GCFrameRegistration*    m_pGCFrameRegistrations;
     PTR_VOID                m_pStackLow;
     PTR_VOID                m_pStackHigh;
-    PTR_UInt8               m_pTEB;                                 // Pointer to OS TEB structure for this thread
     EEThreadId              m_threadId;                             // OS thread ID
     PTR_VOID                m_pThreadStressLog;                     // pointer to head of thread's StressLogChunks
     NATIVE_CONTEXT*         m_interruptedContext;                   // context for an asynchronously interrupted thread.
@@ -105,7 +96,6 @@ struct ThreadBuffer
 #ifdef FEATURE_GC_STRESS
     uint32_t                m_uRand;                                // current per-thread random number
 #endif // FEATURE_GC_STRESS
-
 };
 
 struct ReversePInvokeFrame
@@ -182,13 +172,16 @@ class Thread : private ThreadBuffer
     void GcScanRootsWorker(void * pfnEnumCallback, void * pvCallbackData, StackFrameIterator & sfIter);
 
 public:
-
-    void Detach(); // First phase of thread destructor, executed with thread store lock taken
-    void Destroy(); // Second phase of thread destructor, executed without thread store lock taken
+    // First phase of thread destructor, disposes stuff related to GC.
+    // Executed with thread store lock taken so GC cannot happen.
+    void Detach();
+    // Second phase of thread destructor.
+    // Executed without thread store lock taken.
+    void Destroy();
 
     bool                IsInitialized();
 
-    gc_alloc_context *  GetAllocContext();  // @TODO: I would prefer to not expose this in this way
+    gc_alloc_context *  GetAllocContext();
 
 #ifndef DACCESS_COMPILE
     uint64_t            GetPalThreadIdForLogging();
@@ -212,11 +205,7 @@ class Thread : private ThreadBuffer
     void                SetSuppressGcStress();
     void                ClearSuppressGcStress();
     bool                IsWithinStackBounds(PTR_VOID p);
-
     void                GetStackBounds(PTR_VOID * ppStackLow, PTR_VOID * ppStackHigh);
-
-    PTR_UInt8           GetThreadLocalStorage(uint32_t uTlsIndex, uint32_t uTlsStartOffset);
-
     void                PushExInfo(ExInfo * pExInfo);
     void                ValidateExInfoPop(ExInfo * pExInfo, void * limitSP);
     void                ValidateExInfoStack();