diff --git a/src/coreclr/vm/ceemain.cpp b/src/coreclr/vm/ceemain.cpp index d7d5257ea18366..b0ff3821931306 100644 --- a/src/coreclr/vm/ceemain.cpp +++ b/src/coreclr/vm/ceemain.cpp @@ -396,6 +396,12 @@ BOOL g_singleVersionHosting; typedef BOOL(WINAPI* PINITIALIZECONTEXT2)(PVOID Buffer, DWORD ContextFlags, PCONTEXT* Context, PDWORD ContextLength, ULONG64 XStateCompactionMask); PINITIALIZECONTEXT2 g_pfnInitializeContext2 = NULL; +static BOOLEAN WINAPI lRtlDllShutdownInProgressFallback() +{ + return g_fProcessDetach; +} +PRTLDLLSHUTDOWNINPROGRESS g_pfnRtlDllShutdownInProgress = &lRtlDllShutdownInProgressFallback; + #ifdef TARGET_X86 typedef VOID(__cdecl* PRTLRESTORECONTEXT)(PCONTEXT ContextRecord, struct _EXCEPTION_RECORD* ExceptionRecord); PRTLRESTORECONTEXT g_pfnRtlRestoreContext = NULL; @@ -406,8 +412,12 @@ void InitializeOptionalWindowsAPIPointers() HMODULE hm = GetModuleHandleW(_T("kernel32.dll")); g_pfnInitializeContext2 = (PINITIALIZECONTEXT2)GetProcAddress(hm, "InitializeContext2"); -#ifdef TARGET_X86 hm = GetModuleHandleW(_T("ntdll.dll")); + PRTLDLLSHUTDOWNINPROGRESS pfn = (PRTLDLLSHUTDOWNINPROGRESS)GetProcAddress(hm, "RtlDllShutdownInProgress"); + if (pfn != NULL) + g_pfnRtlDllShutdownInProgress = pfn; + +#ifdef TARGET_X86 g_pfnRtlRestoreContext = (PRTLRESTORECONTEXT)GetProcAddress(hm, "RtlRestoreContext"); #endif //TARGET_X86 } @@ -1159,11 +1169,6 @@ void STDMETHODCALLTYPE EEShutDownHelper(BOOL fIsDllUnloading) Thread * pThisThread = GetThreadNULLOk(); #endif - // If the process is detaching then set the global state. - // This is used to get around FreeLibrary problems. - if(fIsDllUnloading) - g_fProcessDetach = true; - if (IsDbgHelperSpecialThread()) { // Our debugger helper thread does not allow Thread object to be set up. @@ -1478,17 +1483,6 @@ BOOL IsThreadInSTA() // Actual shut down logic is factored out to EEShutDownHelper which may be called // directly by EEShutDown, or indirectly on another thread (see code:#STAShutDown). // -// In order that callees may also know the value of fIsDllUnloading, EEShutDownHelper -// sets g_fProcessDetach = fIsDllUnloading, and g_fProcessDetach may then be retrieved -// via code:IsAtProcessExit. -// -// NOTE 1: Actually, g_fProcessDetach is set to TRUE if fIsDllUnloading is TRUE. But -// g_fProcessDetach doesn't appear to be explicitly set to FALSE. (Apparently -// g_fProcessDetach is implicitly initialized to FALSE as clr.dll is loaded.) -// -// NOTE 2: EEDllMain(DLL_PROCESS_DETACH) already sets g_fProcessDetach to TRUE, so it -// appears EEShutDownHelper doesn't have to. -// void STDMETHODCALLTYPE EEShutDown(BOOL fIsDllUnloading) { CONTRACTL { @@ -1729,25 +1723,13 @@ struct TlsDestructionMonitor thread->m_pFrame = FRAME_TOP; GCX_COOP_NO_DTOR_END(); } -#ifdef _DEBUG - BOOL oldGCOnTransitionsOK = thread->m_GCOnTransitionsOK; - thread->m_GCOnTransitionsOK = FALSE; -#endif - if (!IsAtProcessExit() && !g_fEEShutDown) - { - GCX_COOP_NO_DTOR(); - FreeThreadStaticData(&t_ThreadStatics, thread); - GCX_COOP_NO_DTOR_END(); - } -#ifdef _DEBUG - thread->m_GCOnTransitionsOK = oldGCOnTransitionsOK; -#endif + thread->DetachThread(TRUE); } else { // Since we don't actually cleanup the TLS data along this path, verify that it is already cleaned up - AssertThreadStaticDataFreed(&t_ThreadStatics); + AssertThreadStaticDataFreed(); } ThreadDetaching(); diff --git a/src/coreclr/vm/threads.cpp b/src/coreclr/vm/threads.cpp index abd6bac8050f9f..f8c9aa7dc9cc02 100644 --- a/src/coreclr/vm/threads.cpp +++ b/src/coreclr/vm/threads.cpp @@ -854,22 +854,6 @@ void DestroyThread(Thread *th) th->UnmarkThreadForAbort(); } - // Clear any outstanding stale EH state that maybe still active on the thread. -#ifdef FEATURE_EH_FUNCLETS - ExceptionTracker::PopTrackers((void*)-1); -#else // !FEATURE_EH_FUNCLETS -#ifdef TARGET_X86 - PTR_ThreadExceptionState pExState = th->GetExceptionState(); - if (pExState->IsExceptionInProgress()) - { - GCX_COOP(); - pExState->GetCurrentExceptionTracker()->UnwindExInfo((void *)-1); - } -#else // !TARGET_X86 -#error Unsupported platform -#endif // TARGET_X86 -#endif // FEATURE_EH_FUNCLETS - if (g_fEEShutDown == 0) { th->SetThreadState(Thread::TS_ReportDead); @@ -890,22 +874,6 @@ HRESULT Thread::DetachThread(BOOL fDLLThreadDetach) STATIC_CONTRACT_NOTHROW; STATIC_CONTRACT_GC_NOTRIGGER; - // Clear any outstanding stale EH state that maybe still active on the thread. -#ifdef FEATURE_EH_FUNCLETS - ExceptionTracker::PopTrackers((void*)-1); -#else // !FEATURE_EH_FUNCLETS -#ifdef TARGET_X86 - PTR_ThreadExceptionState pExState = GetExceptionState(); - if (pExState->IsExceptionInProgress()) - { - GCX_COOP(); - pExState->GetCurrentExceptionTracker()->UnwindExInfo((void *)-1); - } -#else // !TARGET_X86 -#error Unsupported platform -#endif // TARGET_X86 -#endif // FEATURE_EH_FUNCLETS - #ifdef FEATURE_COMINTEROP IErrorInfo *pErrorInfo; // Avoid calling GetErrorInfo() if ole32 has already executed the DLL_THREAD_DETACH, @@ -962,19 +930,7 @@ HRESULT Thread::DetachThread(BOOL fDLLThreadDetach) m_ThreadHandleForClose = hThread; } - if (GCHeapUtilities::IsGCHeapInitialized()) - { - // If the GC heap is initialized, we need to fix the alloc context for this detaching thread. - GCX_COOP(); - // GetTotalAllocatedBytes reads dead_threads_non_alloc_bytes, but will suspend EE, being in COOP mode we cannot race with that - // however, there could be other threads terminating and doing the same Add. - InterlockedExchangeAdd64((LONG64*)&dead_threads_non_alloc_bytes, t_runtime_thread_locals.alloc_context.alloc_limit - t_runtime_thread_locals.alloc_context.alloc_ptr); - GCHeapUtilities::GetGCHeap()->FixAllocContext(&t_runtime_thread_locals.alloc_context, NULL, NULL); - t_runtime_thread_locals.alloc_context.init(); // re-initialize the context. - - // Clear out the alloc context pointer for this thread. When TLS is gone, this pointer will point into freed memory. - m_pRuntimeThreadLocals = nullptr; - } + SynchronousThreadCleanup(); // We need to make sure that TLS are touched last here. SetThread(NULL); @@ -2774,6 +2730,53 @@ void Thread::CleanupCOMState() } #endif // FEATURE_COMINTEROP_APARTMENT_SUPPORT +// Thread cleanup that must be run synchronously before the thread is destroyed. +// Failure to run this cleanup will result into dangling pointers and memory leaks. +void Thread::SynchronousThreadCleanup() +{ + CONTRACTL { + NOTHROW; + GC_TRIGGERS; + } + CONTRACTL_END; + + // Skip the cleanup during process exit. Switch to cooperative mode can deadlock during process exit on Windows. + if (IsAtProcessExit()) + return; + + GCX_COOP(); + + // Clear any outstanding stale EH state that maybe still active on the thread. +#ifdef FEATURE_EH_FUNCLETS + ExceptionTracker::PopTrackers((void*)-1); +#else // !FEATURE_EH_FUNCLETS + PTR_ThreadExceptionState pExState = th->GetExceptionState(); + if (pExState->IsExceptionInProgress()) + { + pExState->GetCurrentExceptionTracker()->UnwindExInfo((void *)-1); + } +#endif // FEATURE_EH_FUNCLETS + + if (m_ThreadLocalDataPtr != NULL) + { + FreeThreadStaticData(this); + m_ThreadLocalDataPtr = NULL; + } + + if (GCHeapUtilities::IsGCHeapInitialized()) + { + // If the GC heap is initialized, we need to fix the alloc context for this detaching thread. + // GetTotalAllocatedBytes reads dead_threads_non_alloc_bytes, but will suspend EE, being in COOP mode we cannot race with that + // however, there could be other threads terminating and doing the same Add. + InterlockedExchangeAdd64((LONG64*)&dead_threads_non_alloc_bytes, t_runtime_thread_locals.alloc_context.alloc_limit - t_runtime_thread_locals.alloc_context.alloc_ptr); + GCHeapUtilities::GetGCHeap()->FixAllocContext(&t_runtime_thread_locals.alloc_context, NULL, NULL); + t_runtime_thread_locals.alloc_context.init(); // re-initialize the context. + + // Clear out the alloc context pointer for this thread. When TLS is gone, this pointer will point into freed memory. + m_pRuntimeThreadLocals = nullptr; + } +} + // See general comments on thread destruction (code:#threadDestruction) above. void Thread::OnThreadTerminate(BOOL holdingLock) { @@ -2809,6 +2812,11 @@ void Thread::OnThreadTerminate(BOOL holdingLock) } #endif // FEATURE_COMINTEROP_APARTMENT_SUPPORT + if (this == GetThreadNULLOk()) + { + SynchronousThreadCleanup(); + } + if (g_fEEShutDown != 0) { // We have started shutdown. Not safe to touch CLR state. @@ -2836,24 +2844,8 @@ void Thread::OnThreadTerminate(BOOL holdingLock) // Destroy the LastThrown handle (and anything that violates the above assert). SafeSetThrowables(NULL); - // Free all structures related to thread statics for this thread - DeleteThreadStaticData(); - - } - - if (GCHeapUtilities::IsGCHeapInitialized()) - { - // Guaranteed to NOT be a shutdown case, because we tear down the heap before - // we tear down any threads during shutdown. - if (ThisThreadID == CurrentThreadID && GetAllocContext() != nullptr) - { - GCX_COOP(); - // GetTotalAllocatedBytes reads dead_threads_non_alloc_bytes, but will suspend EE, being in COOP mode we cannot race with that - // however, there could be other threads terminating and doing the same Add. - InterlockedExchangeAdd64((LONG64*)&dead_threads_non_alloc_bytes, GetAllocContext()->alloc_limit - GetAllocContext()->alloc_ptr); - GCHeapUtilities::GetGCHeap()->FixAllocContext(GetAllocContext(), NULL, NULL); - GetAllocContext()->init(); // re-initialize the context. - } + // Free loader allocator structures related to this thread + FreeLoaderAllocatorHandlesForTLSData(this); } // We switch a thread to dead when it has finished doing useful work. But it @@ -2950,14 +2942,6 @@ void Thread::OnThreadTerminate(BOOL holdingLock) // If nobody else is holding onto the thread, we may destruct it here: ULONG oldCount = DecExternalCount(TRUE); - // If we are shutting down the process, we only have one thread active in the - // system. So we can disregard all the reasons that hold this thread alive -- - // TLS is about to be reclaimed anyway. - if (IsAtProcessExit()) - while (oldCount > 0) - { - oldCount = DecExternalCount(TRUE); - } // ASSUME THAT THE THREAD IS DELETED, FROM HERE ON @@ -2985,7 +2969,7 @@ void Thread::OnThreadTerminate(BOOL holdingLock) if (!holdingLock) { LOG((LF_SYNC, INFO3, "OnThreadTerminate releasing lock\n")); - ThreadSuspend::UnlockThreadStore(ThisThreadID == CurrentThreadID); + ThreadSuspend::UnlockThreadStore(); } } } @@ -5130,7 +5114,7 @@ void ThreadStore::UnlockThreadStore() // The actual implementation is in ThreadSuspend class since it is coupled // with thread suspension logic - ThreadSuspend::UnlockThreadStore(FALSE, ThreadSuspend::SUSPEND_OTHER); + ThreadSuspend::UnlockThreadStore(ThreadSuspend::SUSPEND_OTHER); } // AddThread adds 'newThread' to m_ThreadList @@ -7590,32 +7574,6 @@ Frame * Thread::NotifyFrameChainOfExceptionUnwind(Frame* pStartFrame, LPVOID pvL return pFrame; } -//+---------------------------------------------------------------------------- -// -// Method: Thread::DeleteThreadStaticData private -// -// Synopsis: Delete the static data for each appdomain that this thread -// visited. -// -// -//+---------------------------------------------------------------------------- - -void Thread::DeleteThreadStaticData() -{ - CONTRACTL { - NOTHROW; - GC_NOTRIGGER; - MODE_COOPERATIVE; - } - CONTRACTL_END; - - FreeLoaderAllocatorHandlesForTLSData(this); - if (!IsAtProcessExit() && !g_fEEShutDown) - { - FreeThreadStaticData(m_ThreadLocalDataPtr, this); - } -} - OBJECTREF Thread::GetCulture(BOOL bUICulture) { CONTRACTL { diff --git a/src/coreclr/vm/threads.h b/src/coreclr/vm/threads.h index c1ee8e7585bbff..11ce60fb2f5fc4 100644 --- a/src/coreclr/vm/threads.h +++ b/src/coreclr/vm/threads.h @@ -1180,6 +1180,8 @@ class Thread void BaseWinRTUninitialize(); #endif // FEATURE_COMINTEROP_APARTMENT_SUPPORT + void SynchronousThreadCleanup(); + void OnThreadTerminate(BOOL holdingLock); static void CleanupDetachedThreads(); @@ -3339,12 +3341,6 @@ class Thread SpinLock m_TlsSpinLock; PTR_ThreadLocalData GetThreadLocalDataPtr() { LIMITED_METHOD_DAC_CONTRACT; return m_ThreadLocalDataPtr; } -public: - - // Called during Thread death to clean up all structures - // associated with thread statics - void DeleteThreadStaticData(); - private: TailCallTls m_tailCallTls; diff --git a/src/coreclr/vm/threadstatics.cpp b/src/coreclr/vm/threadstatics.cpp index 23c38f65818635..b9dfed148d3863 100644 --- a/src/coreclr/vm/threadstatics.cpp +++ b/src/coreclr/vm/threadstatics.cpp @@ -408,22 +408,21 @@ void FreeLoaderAllocatorHandlesForTLSData(Thread *pThread) } } -void AssertThreadStaticDataFreed(ThreadLocalData *pThreadLocalData) +void AssertThreadStaticDataFreed() { LIMITED_METHOD_CONTRACT; - if (!IsAtProcessExit() && !g_fEEShutDown) - { - _ASSERTE(pThreadLocalData->pThread == NULL); - _ASSERTE(pThreadLocalData->pCollectibleTlsArrayData == NULL); - _ASSERTE(pThreadLocalData->cCollectibleTlsData == 0); - _ASSERTE(pThreadLocalData->pNonCollectibleTlsArrayData == NULL); - _ASSERTE(pThreadLocalData->cNonCollectibleTlsData == 0); - _ASSERTE(pThreadLocalData->pInFlightData == NULL); - } + ThreadLocalData *pThreadLocalData = &t_ThreadStatics; + + _ASSERTE(pThreadLocalData->pThread == NULL); + _ASSERTE(pThreadLocalData->pCollectibleTlsArrayData == NULL); + _ASSERTE(pThreadLocalData->cCollectibleTlsData == 0); + _ASSERTE(pThreadLocalData->pNonCollectibleTlsArrayData == NULL); + _ASSERTE(pThreadLocalData->cNonCollectibleTlsData == 0); + _ASSERTE(pThreadLocalData->pInFlightData == NULL); } -void FreeThreadStaticData(ThreadLocalData *pThreadLocalData, Thread* pThread) +void FreeThreadStaticData(Thread* pThread) { CONTRACTL { NOTHROW; @@ -432,47 +431,34 @@ void FreeThreadStaticData(ThreadLocalData *pThreadLocalData, Thread* pThread) } CONTRACTL_END; - if (pThreadLocalData == NULL) - return; - - if (!IsAtProcessExit() && !g_fEEShutDown) - { - SpinLockHolder spinLock(&pThread->m_TlsSpinLock); - - if (pThreadLocalData->pThread == NULL) - { - return; - } - - pThreadLocalData = pThread->m_ThreadLocalDataPtr; + SpinLockHolder spinLock(&pThread->m_TlsSpinLock); - if (pThreadLocalData == NULL) - return; + ThreadLocalData *pThreadLocalData = &t_ThreadStatics; - for (int32_t iTlsSlot = 0; iTlsSlot < pThreadLocalData->cCollectibleTlsData; ++iTlsSlot) + for (int32_t iTlsSlot = 0; iTlsSlot < pThreadLocalData->cCollectibleTlsData; ++iTlsSlot) + { + if (!IsHandleNullUnchecked(pThreadLocalData->pCollectibleTlsArrayData[iTlsSlot])) { - if (!IsHandleNullUnchecked(pThreadLocalData->pCollectibleTlsArrayData[iTlsSlot])) - { - DestroyLongWeakHandle(pThreadLocalData->pCollectibleTlsArrayData[iTlsSlot]); - } + DestroyLongWeakHandle(pThreadLocalData->pCollectibleTlsArrayData[iTlsSlot]); } + } - delete[] (uint8_t*)pThreadLocalData->pCollectibleTlsArrayData; + delete[] (uint8_t*)pThreadLocalData->pCollectibleTlsArrayData; - pThreadLocalData->pCollectibleTlsArrayData = 0; - pThreadLocalData->cCollectibleTlsData = 0; - pThreadLocalData->pNonCollectibleTlsArrayData = 0; - pThreadLocalData->cNonCollectibleTlsData = 0; + pThreadLocalData->pCollectibleTlsArrayData = 0; + pThreadLocalData->cCollectibleTlsData = 0; + pThreadLocalData->pNonCollectibleTlsArrayData = 0; + pThreadLocalData->cNonCollectibleTlsData = 0; - while (pThreadLocalData->pInFlightData != NULL) - { - InFlightTLSData* pInFlightData = pThreadLocalData->pInFlightData; - pThreadLocalData->pInFlightData = pInFlightData->pNext; - delete pInFlightData; - } - pThreadLocalData->pThread->m_ThreadLocalDataPtr = NULL; - VolatileStoreWithoutBarrier(&pThreadLocalData->pThread, (Thread*)NULL); + while (pThreadLocalData->pInFlightData != NULL) + { + InFlightTLSData* pInFlightData = pThreadLocalData->pInFlightData; + pThreadLocalData->pInFlightData = pInFlightData->pNext; + delete pInFlightData; } + + _ASSERTE(pThreadLocalData->pThread == pThread); + pThreadLocalData->pThread = NULL; } void SetTLSBaseValue(TADDR *ppTLSBaseAddress, TADDR pTLSBaseAddress, bool useGCBarrierInsteadOfHandleStore) @@ -554,7 +540,6 @@ void* GetThreadLocalStaticBase(TLSIndex index) if (cCollectibleTlsData <= index.GetIndexOffset()) { // Grow the underlying TLS array - SpinLockHolder spinLock(&t_ThreadStatics.pThread->m_TlsSpinLock); int32_t newcCollectibleTlsData = index.GetIndexOffset() + 8; // Leave a bit of margin OBJECTHANDLE* pNewTLSArrayData = new OBJECTHANDLE[newcCollectibleTlsData]; memset(pNewTLSArrayData, 0, newcCollectibleTlsData * sizeof(OBJECTHANDLE)); @@ -570,7 +555,6 @@ void* GetThreadLocalStaticBase(TLSIndex index) if (isCollectible && t_ThreadStatics.pThread->cLoaderHandles <= index.GetIndexOffset()) { // Grow the underlying TLS array - SpinLockHolder spinLock(&t_ThreadStatics.pThread->m_TlsSpinLock); int32_t cNewTLSLoaderHandles = index.GetIndexOffset() + 8; // Leave a bit of margin size_t cbNewTLSLoaderHandles = sizeof(LOADERHANDLE) * cNewTLSLoaderHandles; LOADERHANDLE* pNewTLSLoaderHandles = new LOADERHANDLE[cNewTLSLoaderHandles]; @@ -608,7 +592,6 @@ void* GetThreadLocalStaticBase(TLSIndex index) gcBaseAddresses.pTLSBaseAddress = dac_cast(OBJECTREFToObject(ObjectFromHandle(pInFlightData->hTLSData))); if (pMT->IsClassInited()) { - SpinLockHolder spinLock(&t_ThreadStatics.pThread->m_TlsSpinLock); SetTLSBaseValue(gcBaseAddresses.ppTLSBaseAddress, gcBaseAddresses.pTLSBaseAddress, staticIsNonCollectible); *ppOldNextPtr = pInFlightData->pNext; delete pInFlightData; @@ -677,7 +660,6 @@ void* GetThreadLocalStaticBase(TLSIndex index) } else { - SpinLockHolder spinLock(&t_ThreadStatics.pThread->m_TlsSpinLock); pInFlightData->pNext = t_ThreadStatics.pInFlightData; StoreObjectInHandle(pInFlightData->hTLSData, gc.tlsEntry); t_ThreadStatics.pInFlightData = pInFlightData; diff --git a/src/coreclr/vm/threadstatics.h b/src/coreclr/vm/threadstatics.h index 0422629e05f9cc..3ab3806cbf952a 100644 --- a/src/coreclr/vm/threadstatics.h +++ b/src/coreclr/vm/threadstatics.h @@ -329,8 +329,8 @@ PTR_MethodTable LookupMethodTableForThreadStaticKnownToBeAllocated(TLSIndex inde void InitializeThreadStaticData(); void InitializeCurrentThreadsStaticData(Thread* pThread); void FreeLoaderAllocatorHandlesForTLSData(Thread* pThread); -void FreeThreadStaticData(ThreadLocalData *pThreadLocalData, Thread* pThread); -void AssertThreadStaticDataFreed(ThreadLocalData *pThreadLocalData); +void FreeThreadStaticData(Thread* pThread); +void AssertThreadStaticDataFreed(); void GetTLSIndexForThreadStatic(MethodTable* pMT, bool gcStatic, TLSIndex* pIndex, uint32_t bytesNeeded); void FreeTLSIndicesForLoaderAllocator(LoaderAllocator *pLoaderAllocator); void* GetThreadLocalStaticBase(TLSIndex index); diff --git a/src/coreclr/vm/vars.hpp b/src/coreclr/vm/vars.hpp index 30e9e60d691558..5aa48135ae30e9 100644 --- a/src/coreclr/vm/vars.hpp +++ b/src/coreclr/vm/vars.hpp @@ -459,12 +459,24 @@ GVAL_DECL(bool, g_metadataUpdatesApplied); #endif EXTERN bool g_fManagedAttach; +#ifdef HOST_WINDOWS +typedef BOOLEAN (WINAPI* PRTLDLLSHUTDOWNINPROGRESS)(); +EXTERN PRTLDLLSHUTDOWNINPROGRESS g_pfnRtlDllShutdownInProgress; +#endif + // Indicates whether we're executing shut down as a result of DllMain // (DLL_PROCESS_DETACH). See comments at code:EEShutDown for details. -inline BOOL IsAtProcessExit() +inline bool IsAtProcessExit() { SUPPORTS_DAC; +#if defined(DACCESS_COMPILE) || !defined(HOST_WINDOWS) return g_fProcessDetach; +#else + // RtlDllShutdownInProgress provides more accurace information about whether the process is shutting down. + // Use it if it is available to avoid shutdown deadlocks. + // https://learn.microsoft.com/windows/win32/devnotes/rtldllshutdowninprogress + return g_pfnRtlDllShutdownInProgress(); +#endif } enum FWStatus