From c2ee2969bfcd38acfdf40e4e56f4bd5ebe6b6dd7 Mon Sep 17 00:00:00 2001 From: Steve Pfister Date: Tue, 1 Sep 2026 18:49:29 -0400 Subject: [PATCH 1/3] Release ThreadStore lock before shutdown wait Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/ceemain.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/coreclr/vm/ceemain.cpp b/src/coreclr/vm/ceemain.cpp index 188cdb9abd50b4..2f6d7fc8693e57 100644 --- a/src/coreclr/vm/ceemain.cpp +++ b/src/coreclr/vm/ceemain.cpp @@ -1175,6 +1175,14 @@ void WaitForEndOfShutdown() CONTRACT_VIOLATION(GCViolation); Thread *pThread = GetThreadNULLOk(); + + // This method never returns, so an outer ThreadStore lock holder cannot release + // the lock. Release it here so shutdown work cannot be permanently blocked. + if (ThreadStore::HoldingThreadStore(pThread)) + { + ThreadSuspend::UnlockThreadStore(); + } + // After a thread is blocked in WaitForEndOfShutdown, the thread should not enter runtime again, // and block at WaitForEndOfShutdown again. if (pThread) From 2d36cb0535ce07fc4839d753f05d1f76e130fa99 Mon Sep 17 00:00:00 2001 From: Steve Pfister Date: Wed, 2 Sep 2026 07:05:58 -0400 Subject: [PATCH 2/3] Move ThreadStore release to debugger shutdown path Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/coreclr/debug/ee/debugger.cpp | 10 ++++++++++ src/coreclr/vm/ceemain.cpp | 7 ------- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/coreclr/debug/ee/debugger.cpp b/src/coreclr/debug/ee/debugger.cpp index b7cfc4d2278fdd..8c9f9213114f05 100644 --- a/src/coreclr/debug/ee/debugger.cpp +++ b/src/coreclr/debug/ee/debugger.cpp @@ -372,6 +372,16 @@ void Debugger::DoNotCallDirectlyPrivateLock(void) // We need to be in preemptive to block for shutdown, so we don't do this block in Coop mode. // Fortunately, it's safe to take this lock in coop mode because we know the thread can't block // on anything interesting because we're in a GC-forbid region (see crst flags). + if (!IsFinalizerThread() && + !IsDbgHelperSpecialThread() && + !IsShutdownSpecialThread() && + !IsGCSpecialThread() && + ThreadStore::HoldingThreadStore(pThread)) + { + // This thread is about to block forever, so the outer holder cannot release the lock. + ThreadSuspend::UnlockThreadStore(); + } + m_mutex.ReleaseAndBlockForShutdownIfNotSpecialThread(); } diff --git a/src/coreclr/vm/ceemain.cpp b/src/coreclr/vm/ceemain.cpp index 2f6d7fc8693e57..6bac23b30d6949 100644 --- a/src/coreclr/vm/ceemain.cpp +++ b/src/coreclr/vm/ceemain.cpp @@ -1176,13 +1176,6 @@ void WaitForEndOfShutdown() Thread *pThread = GetThreadNULLOk(); - // This method never returns, so an outer ThreadStore lock holder cannot release - // the lock. Release it here so shutdown work cannot be permanently blocked. - if (ThreadStore::HoldingThreadStore(pThread)) - { - ThreadSuspend::UnlockThreadStore(); - } - // After a thread is blocked in WaitForEndOfShutdown, the thread should not enter runtime again, // and block at WaitForEndOfShutdown again. if (pThread) From 9dfd990059884a65802ba9a2a1172e28008984fd Mon Sep 17 00:00:00 2001 From: Steve Pfister Date: Wed, 2 Sep 2026 09:32:27 -0400 Subject: [PATCH 3/3] Move shutdown blocking helper to debugger Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/coreclr/debug/ee/debugger.cpp | 42 +++++++++++++++++++++++-------- src/coreclr/debug/ee/debugger.h | 1 + src/coreclr/vm/ceemain.cpp | 1 - src/coreclr/vm/crst.cpp | 37 --------------------------- src/coreclr/vm/crst.h | 17 ------------- 5 files changed, 32 insertions(+), 66 deletions(-) diff --git a/src/coreclr/debug/ee/debugger.cpp b/src/coreclr/debug/ee/debugger.cpp index 8c9f9213114f05..30cf2c29604744 100644 --- a/src/coreclr/debug/ee/debugger.cpp +++ b/src/coreclr/debug/ee/debugger.cpp @@ -303,6 +303,36 @@ HelperCanary * Debugger::GetCanary() return g_pRCThread->GetCanary(); } +void Debugger::ReleaseDebuggerLockAndBlockForShutdownIfNotSpecialThread(Thread *pThread) +{ + CONTRACTL + { + NOTHROW; + MODE_ANY; + GC_NOTRIGGER; + PRECONDITION(m_mutex.OwnedByCurrentThread()); + } + CONTRACTL_END; + + if ((t_ThreadType & (ThreadType_Finalizer|ThreadType_DbgHelper|ThreadType_Shutdown|ThreadType_GC)) == 0) + { + m_mutex.Leave(); + + // Debugger event sending acquires the ThreadStore lock before the debugger lock. + // Since this thread is about to block forever, release the ThreadStore lock explicitly. + if (ThreadStore::HoldingThreadStore(pThread)) + { + ThreadSuspend::UnlockThreadStore(); + } + + GCX_ASSERT_PREEMP(); + + WaitForEndOfShutdown(); + __SwitchToThread(INFINITE, CALLER_LIMITS_SPINNING); + _ASSERTE(!"Can not reach here"); + } +} + // IMPORTANT!!!!! // Do not call Lock and Unlock directly. Because you might not unlock // if exception takes place. Use DebuggerLockHolder instead!!! @@ -372,17 +402,7 @@ void Debugger::DoNotCallDirectlyPrivateLock(void) // We need to be in preemptive to block for shutdown, so we don't do this block in Coop mode. // Fortunately, it's safe to take this lock in coop mode because we know the thread can't block // on anything interesting because we're in a GC-forbid region (see crst flags). - if (!IsFinalizerThread() && - !IsDbgHelperSpecialThread() && - !IsShutdownSpecialThread() && - !IsGCSpecialThread() && - ThreadStore::HoldingThreadStore(pThread)) - { - // This thread is about to block forever, so the outer holder cannot release the lock. - ThreadSuspend::UnlockThreadStore(); - } - - m_mutex.ReleaseAndBlockForShutdownIfNotSpecialThread(); + ReleaseDebuggerLockAndBlockForShutdownIfNotSpecialThread(pThread); } diff --git a/src/coreclr/debug/ee/debugger.h b/src/coreclr/debug/ee/debugger.h index 3adb7ff084804f..48959f4815c735 100644 --- a/src/coreclr/debug/ee/debugger.h +++ b/src/coreclr/debug/ee/debugger.h @@ -2288,6 +2288,7 @@ class Debugger : public DebugInterface private: void DoNotCallDirectlyPrivateLock(void); void DoNotCallDirectlyPrivateUnlock(void); + void ReleaseDebuggerLockAndBlockForShutdownIfNotSpecialThread(Thread *pThread); // This function gets the jit debugger launched and waits for the native attach to complete // Make sure you called PreJitAttach and it returned TRUE before you call this diff --git a/src/coreclr/vm/ceemain.cpp b/src/coreclr/vm/ceemain.cpp index 6bac23b30d6949..188cdb9abd50b4 100644 --- a/src/coreclr/vm/ceemain.cpp +++ b/src/coreclr/vm/ceemain.cpp @@ -1175,7 +1175,6 @@ void WaitForEndOfShutdown() CONTRACT_VIOLATION(GCViolation); Thread *pThread = GetThreadNULLOk(); - // After a thread is blocked in WaitForEndOfShutdown, the thread should not enter runtime again, // and block at WaitForEndOfShutdown again. if (pThread) diff --git a/src/coreclr/vm/crst.cpp b/src/coreclr/vm/crst.cpp index b43370b5484238..12e4a96d13ca10 100644 --- a/src/coreclr/vm/crst.cpp +++ b/src/coreclr/vm/crst.cpp @@ -77,43 +77,6 @@ void CrstBase::Destroy() ResetFlags(); } -extern void WaitForEndOfShutdown(); - -//----------------------------------------------------------------- -// If we're in shutdown (as determined by caller since each lock needs its -// own shutdown flag) and this is a non-special thread (not helper/finalizer/shutdown), -// then release the crst and block forever. -// See the prototype for more details. -//----------------------------------------------------------------- -void CrstBase::ReleaseAndBlockForShutdownIfNotSpecialThread() -{ - CONTRACTL { - NOTHROW; - - // We're almost always MODE_PREEMPTIVE, but if it's a thread suspending for GC, - // then we might be MODE_COOPERATIVE. Fortunately in that case, we don't block on shutdown. - // We assert this below. - MODE_ANY; - GC_NOTRIGGER; - - PRECONDITION(this->OwnedByCurrentThread()); - } - CONTRACTL_END; - - if ((t_ThreadType & (ThreadType_Finalizer|ThreadType_DbgHelper|ThreadType_Shutdown|ThreadType_GC)) == 0) - { - // The process is shutting down. Release the lock and just block forever. - this->Leave(); - - // is this safe to use here since we never return? - GCX_ASSERT_PREEMP(); - - WaitForEndOfShutdown(); - __SwitchToThread(INFINITE, CALLER_LIMITS_SPINNING); - _ASSERTE (!"Can not reach here"); - } -} - #endif // DACCESS_COMPILE diff --git a/src/coreclr/vm/crst.h b/src/coreclr/vm/crst.h index 75018b355342ba..8d035dfe464cd3 100644 --- a/src/coreclr/vm/crst.h +++ b/src/coreclr/vm/crst.h @@ -138,23 +138,6 @@ class CrstBase #endif private: - // Some Crsts have a "shutdown" mode. - // A Crst in shutdown mode can only be taken / released by special - // (the helper / finalizer / shutdown) threads. Any other thread that tries to take - // the a "shutdown" crst will immediately release the Crst and instead just block forever. - // - // This prevents random threads from blocking the special threads from doing finalization on shutdown. - // - // Unfortunately, each Crst needs its own "shutdown" flag because we can't convert all the locks - // into shutdown locks at once. For eg, the TSL needs to suspend the runtime before - // converting to a shutdown lock. But it can't suspend the runtime while holding - // a UNSAFE_ANYMODE lock (such as the debugger-lock). So at least the debugger-lock - // and TSL need to be set separately. - // - // So for such Crsts, it's the caller's responsibility to detect if the crst is in - // shutdown mode, and if so, call this function after enter. - void ReleaseAndBlockForShutdownIfNotSpecialThread(); - // Enter & Leave are deliberately private to force callers to use the // Holder class. If you bypass the Holder class and access these members // directly, your lock is not exception-safe.