Skip to content

Commit

Permalink
Use RtlDllShutdownInProgress to detect process shutdown on Windows (#…
Browse files Browse the repository at this point in the history
…103877)

* Use RtlDllShutdownInProgress to detect process shutdown on Windows

Switching to cooperative mode is not safe during process shutdown on
Windows. Process shutdown can terminate a thread in the middle of the
GC. The shutdown thread deadlocks if it tries to switch to cooperative
mode and wait for the GC to finish in this situation.

Use RtlDllShutdownInProgress Windows API to detect process
shutdown to skip cleanup that has to be done in cooperative mode.

The existing g_fProcessDetach flag is set too late - using this flag to
skip cooperative mode switch would lead to shutdown deadlocks, and the
existing g_fEEShutDown flag is set too early - using this flag to skip
cooperative mode switch would lead to shutdown crashes.

Fixes #103624

* Replace direct g_fProcessDetach checks with a call to IsAtProcessExit
  • Loading branch information
jkotas authored Jul 2, 2024
1 parent 104e88d commit 5044e93
Show file tree
Hide file tree
Showing 25 changed files with 168 additions and 239 deletions.
2 changes: 1 addition & 1 deletion src/coreclr/debug/ee/controller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1109,7 +1109,7 @@ void DebuggerController::DisableAll()
// thus leaving the patchtable in an inconsistent state such that we may fail trying to walk it.
// Since we're exiting anyways, leaving int3 in the code can't harm anybody.
//
if (!g_fProcessDetach)
if (!IsAtProcessExit())
{
HASHFIND f;
for (DebuggerControllerPatch *patch = g_patches->GetFirstPatch(&f);
Expand Down
32 changes: 16 additions & 16 deletions src/coreclr/debug/ee/debugger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ void Debugger::DoNotCallDirectlyPrivateLock(void)


// Lock becomes no-op in late shutdown.
if (g_fProcessDetach)
if (IsAtProcessExit())
{
return;
}
Expand Down Expand Up @@ -430,7 +430,7 @@ void Debugger::DoNotCallDirectlyPrivateUnlock(void)
// Controller lock is "smaller" than debugger lock.


if (!g_fProcessDetach)
if (!IsAtProcessExit())
{
#ifdef _DEBUG
if (m_mutexCount == 1)
Expand Down Expand Up @@ -5035,7 +5035,7 @@ void Debugger::SendSyncCompleteIPCEvent(bool isEESuspendedForGC)
PRECONDITION(ThreadHoldsLock());

// Anyone sending the synccomplete must hold the TSL.
PRECONDITION(ThreadStore::HoldingThreadStore() || g_fProcessDetach);
PRECONDITION(ThreadStore::HoldingThreadStore() || IsAtProcessExit());

// The sync complete is now only sent on a helper thread.
if (!isEESuspendedForGC)
Expand Down Expand Up @@ -5069,7 +5069,7 @@ void Debugger::SendSyncCompleteIPCEvent(bool isEESuspendedForGC)
// We know we're not on the shutdown thread here.
// And we also know we can't block the shutdown thread (b/c it has the TSL and will
// get a free pass through the GC toggles that normally block threads for debugging).
if (g_fProcessDetach)
if (IsAtProcessExit())
{
STRESS_LOG0(LF_CORDB, LL_INFO10000, "D::SSCIPCE: Skipping for shutdown.\n");
return;
Expand Down Expand Up @@ -5304,7 +5304,7 @@ void Debugger::TrapAllRuntimeThreads()
// If we're doing shutdown, then don't bother trying to communicate w/ the RS.
// If we're not the thread doing shutdown, then we may be asynchronously killed by the OS.
// If we are the thread in shutdown, don't TART b/c that may block and do complicated stuff.
if (g_fProcessDetach)
if (IsAtProcessExit())
{
STRESS_LOG0(LF_CORDB, LL_INFO10000, "D::TART: Skipping for shutdown.\n");
return;
Expand Down Expand Up @@ -5355,7 +5355,7 @@ void Debugger::TrapAllRuntimeThreads()
// That means that our helper is not blocked on starting up, thus we can wait infinite on it.
// Thus we don't need to do helper duty if the suspend fails.
bool fShouldDoHelperDuty = !m_pRCThread->IsRCThreadReady() && fSuspended;
if (fShouldDoHelperDuty && !g_fProcessDetach)
if (fShouldDoHelperDuty && !IsAtProcessExit())
{
// In V1.0, we had the assumption that if the helper thread isn't ready yet, then we're in
// a state that SuspendForDebug will succeed on the first try, and thus we'll
Expand Down Expand Up @@ -9139,7 +9139,7 @@ BOOL Debugger::SuspendComplete(bool isEESuspendedForGC)
// If happen on managed thread, it must be doing the helper thread duty.
//

_ASSERTE(ThreadStore::HoldingThreadStore() || g_fProcessDetach);
_ASSERTE(ThreadStore::HoldingThreadStore() || IsAtProcessExit());

// We should be holding debugger lock m_mutex.
_ASSERTE(ThreadHoldsLock());
Expand Down Expand Up @@ -10739,7 +10739,7 @@ bool Debugger::HandleIPCEvent(DebuggerIPCEvent * pEvent)
(pThread != NULL) ? GetThreadIdHelper(pThread) : 0,
debugState));

if (!g_fProcessDetach)
if (!IsAtProcessExit())
{
g_pEEInterface->SetAllDebugState(pThread, debugState);
}
Expand Down Expand Up @@ -10868,7 +10868,7 @@ bool Debugger::HandleIPCEvent(DebuggerIPCEvent * pEvent)
{
pIPCResult->hr = CORDBG_E_NOTREADY;
}
else if (!g_fProcessDetach)
else if (!IsAtProcessExit())
{
//
// Since this pointer is coming from the RS, it may be NULL or something
Expand Down Expand Up @@ -15056,7 +15056,7 @@ HRESULT Debugger::FuncEvalSetup(DebuggerIPCE_FuncEvalInfo *pEvalInfo,
if (pThread->m_State & Thread::TS_AbortRequested)
return CORDBG_E_FUNC_EVAL_BAD_START_POINT;

if (g_fProcessDetach)
if (IsAtProcessExit())
return CORDBG_E_FUNC_EVAL_BAD_START_POINT;

// If there is no guard page on this thread, then we've taken a stack overflow exception and can't run managed
Expand Down Expand Up @@ -15251,7 +15251,7 @@ Debugger::FuncEvalAbort(
"D::FEA: performing UserAbort on thread %#x, id=0x%x\n",
pDE->m_thread, GetThreadIdHelper(pDE->m_thread)));

if (!g_fProcessDetach && !pDE->m_completed)
if (!IsAtProcessExit() && !pDE->m_completed)
{
//
// Perform a stop on the thread that the eval is running on.
Expand Down Expand Up @@ -15317,7 +15317,7 @@ Debugger::FuncEvalRudeAbort(
"D::FEA: performing RudeAbort on thread %#x, id=0x%x\n",
pDE->m_thread, Debugger::GetThreadIdHelper(pDE->m_thread)));

if (!g_fProcessDetach && !pDE->m_completed)
if (!IsAtProcessExit() && !pDE->m_completed)
{
//
// Perform a stop on the thread that the eval is running on.
Expand Down Expand Up @@ -15608,7 +15608,7 @@ void Debugger::DisableDebugger(void)
* This is called in the case that the loader lock is held and so no new
* threads can be spun up to be the helper thread, so the existing thread
* must be the helper thread until a new one can spin up.
* This is also called in the shutdown case (g_fProcessDetach==true) and our
* This is also called in the shutdown case (IsAtProcessExit()==true) and our
* helper may have already been blown away.
***************************************************************************/
void Debugger::DoHelperThreadDuty()
Expand All @@ -15628,7 +15628,7 @@ void Debugger::DoHelperThreadDuty()
// We'll get killed randomly anyways, so not much we can do.

// These assumptions are based off us being called from TART.
_ASSERTE(ThreadStore::HoldingThreadStore() || g_fProcessDetach); // got this from TART
_ASSERTE(ThreadStore::HoldingThreadStore() || IsAtProcessExit()); // got this from TART
_ASSERTE(m_trappingRuntimeThreads); // We're only called from TART.
_ASSERTE(!m_stopped); // we haven't sent the sync-complete yet.

Expand Down Expand Up @@ -16179,7 +16179,7 @@ void Debugger::AcquireDebuggerDataLock(Debugger *pDebugger)
{
WRAPPER_NO_CONTRACT;

if (!g_fProcessDetach)
if (!IsAtProcessExit())
{
pDebugger->GetDebuggerDataLock()->Enter();
}
Expand All @@ -16190,7 +16190,7 @@ void Debugger::ReleaseDebuggerDataLock(Debugger *pDebugger)
{
WRAPPER_NO_CONTRACT;

if (!g_fProcessDetach)
if (!IsAtProcessExit())
{
pDebugger->GetDebuggerDataLock()->Leave();
}
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/debug/ee/debugger.h
Original file line number Diff line number Diff line change
Expand Up @@ -2492,7 +2492,7 @@ class Debugger : public DebugInterface
}
CONTRACTL_END;

if (g_fProcessDetach)
if (IsAtProcessExit())
return true;

if (g_pEEInterface->GetThread())
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/debug/ee/debuggermodule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,8 @@ DebuggerModuleTable::~DebuggerModuleTable()
#ifdef _DEBUG
bool DebuggerModuleTable::ThreadHoldsLock()
{
// In shutdown (g_fProcessDetach), the shutdown thread implicitly holds all locks.
return g_fProcessDetach || g_pDebugger->HasDebuggerDataLock();
// In shutdown (IsAtProcessExit()), the shutdown thread implicitly holds all locks.
return IsAtProcessExit() || g_pDebugger->HasDebuggerDataLock();
}
#endif

Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/debug/ee/frameinfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1875,7 +1875,7 @@ bool ShouldSendUMLeafChain(Thread * pThread)
// If we're in shutodown, don't bother trying to sniff for an UM leaf chain.
// @todo - we'd like to never even be trying to stack trace on shutdown, this
// comes up when we do helper thread duty on shutdown.
if (g_fProcessDetach)
if (IsAtProcessExit())
{
return false;
}
Expand Down
12 changes: 6 additions & 6 deletions src/coreclr/debug/ee/rcthread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -858,7 +858,7 @@ void DebuggerRCThread::MainLoop()
PRECONDITION(m_thread != NULL);
PRECONDITION(ThisIsHelperThreadWorker());
PRECONDITION(IsDbgHelperSpecialThread()); // Can only be called on native debugger helper thread
PRECONDITION((!ThreadStore::HoldingThreadStore()) || g_fProcessDetach);
PRECONDITION((!ThreadStore::HoldingThreadStore()) || IsAtProcessExit());
}
CONTRACTL_END;

Expand Down Expand Up @@ -960,7 +960,7 @@ void DebuggerRCThread::MainLoop()
{

// If they called continue, then we must have released the TSL.
_ASSERTE(!ThreadStore::HoldingThreadStore() || g_fProcessDetach);
_ASSERTE(!ThreadStore::HoldingThreadStore() || IsAtProcessExit());

// Let's release the lock here since runtime is resumed.
debugLockHolderSuspended.Release();
Expand Down Expand Up @@ -1062,7 +1062,7 @@ void DebuggerRCThread::MainLoop()
// We also hold debugger lock the whole time that Runtime is stopped. We will release the debugger lock
// when we receive the Continue event that resumes the runtime.

_ASSERTE(ThreadStore::HoldingThreadStore() || g_fProcessDetach);
_ASSERTE(ThreadStore::HoldingThreadStore() || IsAtProcessExit());
}
else
{
Expand Down Expand Up @@ -1107,7 +1107,7 @@ void DebuggerRCThread::TemporaryHelperThreadMainLoop()
// It should be holding the debugger lock!!!
//
PRECONDITION(m_debugger->ThreadHoldsLock());
PRECONDITION((ThreadStore::HoldingThreadStore()) || g_fProcessDetach);
PRECONDITION((ThreadStore::HoldingThreadStore()) || IsAtProcessExit());
PRECONDITION(ThisIsTempHelperThread());
}
CONTRACTL_END;
Expand Down Expand Up @@ -1177,7 +1177,7 @@ void DebuggerRCThread::TemporaryHelperThreadMainLoop()
if (fWasContinue)
{
// If they called continue, then we must have released the TSL.
_ASSERTE(!ThreadStore::HoldingThreadStore() || g_fProcessDetach);
_ASSERTE(!ThreadStore::HoldingThreadStore() || IsAtProcessExit());

#ifdef _DEBUG
// Always reset the syncSpinCount to 0 on a continue so that we have the maximum number of possible
Expand Down Expand Up @@ -1241,7 +1241,7 @@ void DebuggerRCThread::TemporaryHelperThreadMainLoop()
dwWaitTimeout = INFINITE;

// Note: we hold the thread store lock now and debugger lock...
_ASSERTE(ThreadStore::HoldingThreadStore() || g_fProcessDetach);
_ASSERTE(ThreadStore::HoldingThreadStore() || IsAtProcessExit());

}
}
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/vm/cachelinealloc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ CCacheLineAllocator::~CCacheLineAllocator()
{
if(tempPtr->m_pAddr[i] != NULL)
{
if (!g_fProcessDetach)
if (!IsAtProcessExit())
VFree(tempPtr->m_pAddr[i]);
}
}
Expand Down
54 changes: 18 additions & 36 deletions src/coreclr/vm/ceemain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 RtlDllShutdownInProgressFallback()
{
return g_fProcessDetach;
}
PRTLDLLSHUTDOWNINPROGRESS g_pfnRtlDllShutdownInProgress = &RtlDllShutdownInProgressFallback;

#ifdef TARGET_X86
typedef VOID(__cdecl* PRTLRESTORECONTEXT)(PCONTEXT ContextRecord, struct _EXCEPTION_RECORD* ExceptionRecord);
PRTLRESTORECONTEXT g_pfnRtlRestoreContext = NULL;
Expand All @@ -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
}
Expand Down Expand Up @@ -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.
Expand All @@ -1187,7 +1192,7 @@ void STDMETHODCALLTYPE EEShutDownHelper(BOOL fIsDllUnloading)
// ungracefully. This check is an attempt to recognize that case
// and avoid the impending hang when attempting to get the helper
// thread to do things for us.
if ((g_pDebugInterface != NULL) && g_fProcessDetach)
if ((g_pDebugInterface != NULL) && IsAtProcessExit())
g_pDebugInterface->EarlyHelperThreadDeath();
#endif // DEBUGGING_SUPPORTED

Expand All @@ -1204,7 +1209,7 @@ void STDMETHODCALLTYPE EEShutDownHelper(BOOL fIsDllUnloading)
// Indicate the EE is the shut down phase.
g_fEEShutDown |= ShutDown_Start;

if (!g_fProcessDetach && !g_fFastExitProcess)
if (!IsAtProcessExit() && !g_fFastExitProcess)
{
g_fEEShutDown |= ShutDown_Finalize1;

Expand All @@ -1214,7 +1219,7 @@ void STDMETHODCALLTYPE EEShutDownHelper(BOOL fIsDllUnloading)
}

// Ok. Let's stop the EE.
if (!g_fProcessDetach)
if (!IsAtProcessExit())
{
// Convert key locks into "shutdown" mode. A lock in shutdown mode means:
// - Only the finalizer/helper/shutdown threads will be able to take the lock.
Expand Down Expand Up @@ -1316,7 +1321,7 @@ void STDMETHODCALLTYPE EEShutDownHelper(BOOL fIsDllUnloading)
// are already in use, then skip phase 2. This will happen only when those locks
// are orphaned. In Vista, the penalty for attempting to enter such locks is
// instant process termination.
if (g_fProcessDetach)
if (IsAtProcessExit())
{
// The assert below is a bit too aggressive and has generally brought cases that have been race conditions
// and not easily reproed to validate a bug. A typical race scenario is when there are two threads,
Expand Down Expand Up @@ -1382,7 +1387,7 @@ void STDMETHODCALLTYPE EEShutDownHelper(BOOL fIsDllUnloading)
EX_END_CATCH(SwallowAllExceptions);

ClrFlsClearThreadType(ThreadType_Shutdown);
if (!g_fProcessDetach)
if (!IsAtProcessExit())
{
g_pEEShutDownEvent->Set();
}
Expand Down Expand 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 {
Expand Down Expand Up @@ -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();
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/vm/codeman.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3550,7 +3550,7 @@ void ExecutionManager::CleanupCodeHeaps()
}
CONTRACTL_END;

_ASSERTE (g_fProcessDetach || (GCHeapUtilities::IsGCInProgress() && ::IsGCThread()));
_ASSERTE (IsAtProcessExit() || (GCHeapUtilities::IsGCInProgress() && ::IsGCThread()));

GetEEJitManager()->CleanupCodeHeaps();
}
Expand All @@ -3564,7 +3564,7 @@ void EEJitManager::CleanupCodeHeaps()
}
CONTRACTL_END;

_ASSERTE (g_fProcessDetach || (GCHeapUtilities::IsGCInProgress() && ::IsGCThread()));
_ASSERTE (IsAtProcessExit() || (GCHeapUtilities::IsGCInProgress() && ::IsGCThread()));

// Quick out, don't even take the lock if we have not cleanup to do.
// This is important because ETW takes the CodeHeapLock when it is doing
Expand Down
Loading

0 comments on commit 5044e93

Please sign in to comment.