Skip to content

Add debug assert to detect CRST_DEFAULT locks taken under ANYMODE locks #128912

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
davidwrighton wants to merge 1 commit into
base: main
Choose a base branch
from
+66 −0
Open

Add debug assert to detect CRST_DEFAULT locks taken under ANYMODE locks #128912

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions src/coreclr/vm/crst.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@
#ifndef DACCESS_COMPILE
Volatile<LONG> g_ShutdownCrstUsageCount = 0;

#ifdef _DEBUG
thread_local int t_unsafeAnyModeHeldCount = 0;
#endif

//-----------------------------------------------------------------
// Initialize critical section
//-----------------------------------------------------------------
Expand Down Expand Up @@ -220,6 +224,18 @@ void CrstBase::Enter(INDEBUG(NoLevelCheckFlag noLevelCheckFlag/* = CRST_LEVEL_CH

_ASSERTE(IsCrstInitialized());

#ifdef _DEBUG
// Detect attempts to take a default (GC_TRIGGERS) lock while holding a CRST_UNSAFE_ANYMODE lock.
// Taking a default lock can trigger a GC, which is illegal while a CRST_UNSAFE_ANYMODE lock is held.
if (!(m_dwFlags & (CRST_UNSAFE_ANYMODE | CRST_UNSAFE_COOPGC | CRST_GC_NOTRIGGER_WHEN_TAKEN)))
{
_ASSERTE_MSG(t_unsafeAnyModeHeldCount == 0,
"Taking a CRST_DEFAULT lock while a CRST_UNSAFE_ANYMODE lock or "
"SimpleRWLock COOPERATIVE_OR_PREEMPTIVE lock is held is illegal. "
"The default lock may trigger a GC, which is not allowed under ANYMODE locks.");
}
#endif

BOOL fIsCriticalSectionEnteredAfterFailingOnce = FALSE;

Thread * pThread;
Expand Down Expand Up @@ -260,6 +276,11 @@ void CrstBase::Enter(INDEBUG(NoLevelCheckFlag noLevelCheckFlag/* = CRST_LEVEL_CH

#ifdef _DEBUG
PostEnter();

if (m_dwFlags & CRST_UNSAFE_ANYMODE)
{
t_unsafeAnyModeHeldCount++;
}
Comment thread
davidwrighton marked this conversation as resolved.
#endif

if (fToggle)
Expand All @@ -280,6 +301,11 @@ void CrstBase::Leave()
_ASSERTE(IsCrstInitialized());

#ifdef _DEBUG
if (m_dwFlags & CRST_UNSAFE_ANYMODE)
{
t_unsafeAnyModeHeldCount--;
}
Comment on lines +304 to +307

PreLeave ();
#endif //_DEBUG

Expand Down
6 changes: 6 additions & 0 deletions src/coreclr/vm/crst.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,12 @@
// Total count of Crst lock of the type (Shutdown) that are currently in use
extern Volatile<LONG> g_ShutdownCrstUsageCount;

#ifdef _DEBUG
// Per-thread count of CRST_UNSAFE_ANYMODE (and equivalent) locks currently held.
// Used to detect illegal acquisition of GC_TRIGGERS locks under ANYMODE locks.
extern thread_local int t_unsafeAnyModeHeldCount;
Comment on lines +99 to +101
#endif

// The CRST.
class CrstBase
{
Expand Down
30 changes: 30 additions & 0 deletions src/coreclr/vm/simplerwlock.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,16 @@ void SimpleRWLock::EnterRead()
CheckGCNoTrigger();
#endif //ENABLE_CONTRACTS_IMPL

#ifdef _DEBUG
if (m_gcMode == PREEMPTIVE)
{
_ASSERTE_MSG(t_unsafeAnyModeHeldCount == 0,
"Taking a PREEMPTIVE SimpleRWLock while a COOPERATIVE_OR_PREEMPTIVE lock or "
"CRST_UNSAFE_ANYMODE lock is held is illegal. "
"The PREEMPTIVE lock may trigger a GC, which is not allowed under ANYMODE locks.");
Comment on lines +50 to +53
}
#endif

GCX_MAYBE_PREEMP(m_gcMode == PREEMPTIVE);

#ifdef _DEBUG
Expand Down Expand Up @@ -135,6 +145,16 @@ void SimpleRWLock::EnterWrite()
CheckGCNoTrigger();
#endif //ENABLE_CONTRACTS_IMPL

#ifdef _DEBUG
if (m_gcMode == PREEMPTIVE)
{
_ASSERTE_MSG(t_unsafeAnyModeHeldCount == 0,
"Taking a PREEMPTIVE SimpleRWLock while a COOPERATIVE_OR_PREEMPTIVE lock or "
"CRST_UNSAFE_ANYMODE lock is held is illegal. "
"The PREEMPTIVE lock may trigger a GC, which is not allowed under ANYMODE locks.");
Comment on lines +151 to +154
}
#endif

GCX_MAYBE_PREEMP(m_gcMode == PREEMPTIVE);

#ifdef _DEBUG
Expand Down Expand Up @@ -267,6 +287,11 @@ void SimpleRWLock::PostEnter()
INCONTRACT(pThread->BeginNoTriggerGC(__FILE__, __LINE__));
}
}

if (m_gcMode == COOPERATIVE_OR_PREEMPTIVE)
{
t_unsafeAnyModeHeldCount++;
}
}

//=====================================================================
Expand All @@ -276,6 +301,11 @@ void SimpleRWLock::PreLeave()
{
WRAPPER_NO_CONTRACT;

if (m_gcMode == COOPERATIVE_OR_PREEMPTIVE)
{
t_unsafeAnyModeHeldCount--;
}
Comment on lines +304 to +307

if (m_countNoTriggerGC > 0)
{
DWORD countNoTriggerGC = InterlockedDecrement(&m_countNoTriggerGC);
Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/vm/simplerwlock.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@

#include "threads.h"

#ifdef _DEBUG
extern thread_local int t_unsafeAnyModeHeldCount;
#endif

class SimpleRWLock;

//-------------------------------------------------------------------------------------------
Expand Down
Loading