Skip to content

Commit

Permalink
tweaks
Browse files Browse the repository at this point in the history
  • Loading branch information
VSadov committed May 1, 2024
1 parent 970438f commit a38b54a
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 26 deletions.
2 changes: 1 addition & 1 deletion src/coreclr/vm/threads.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5850,7 +5850,7 @@ BOOL ThreadStore::DbgFindThread(Thread *target)
// return).
//
// Note: we don't actually assert this if
// ThreadStore::TrapReturningThreadsIncrement() updated g_TrapReturningThreads
// ThreadStore::IncrementTrapReturningThreads() updated g_TrapReturningThreads
// between the beginning of this function and the moment of the assert.
// *** The order of evaluation in the if condition is important ***
_ASSERTE(
Expand Down
18 changes: 9 additions & 9 deletions src/coreclr/vm/threads.h
Original file line number Diff line number Diff line change
Expand Up @@ -4152,14 +4152,14 @@ class ThreadStore
}

// If you want to trap threads re-entering the EE (for debugging,
// or Thread.Suspend() or whatever, you need to TrapReturningThreadsIncrement(). When
// you are finished snagging threads, call TrapReturningThreadsDecrement(). This
// or Thread.Suspend() or whatever, you need to IncrementTrapReturningThreads(). When
// you are finished snagging threads, call DecrementTrapReturningThreads(). This
// counts internally.
//
// Of course, you must also fix RareDisablePreemptiveGC to do the right thing
// when the trap occurs.
static void TrapReturningThreadsIncrement();
static void TrapReturningThreadsDecrement();
static void IncrementTrapReturningThreads();
static void DecrementTrapReturningThreads();

static void SetThreadTrapForSuspension();
static void UnsetThreadTrapForSuspension();
Expand Down Expand Up @@ -4327,8 +4327,8 @@ class ThreadStore
};

struct TSSuspendHelper {
static void SetTrap() { ThreadStore::TrapReturningThreadsIncrement(); }
static void UnsetTrap() { ThreadStore::TrapReturningThreadsDecrement(); }
static void SetTrap() { ThreadStore::IncrementTrapReturningThreads(); }
static void UnsetTrap() { ThreadStore::DecrementTrapReturningThreads(); }
};
typedef StateHolder<TSSuspendHelper::SetTrap, TSSuspendHelper::UnsetTrap> TSSuspendHolder;

Expand Down Expand Up @@ -4519,7 +4519,7 @@ inline void Thread::MarkForDebugSuspend(void)
if (!HasThreadState(TS_DebugSuspendPending))
{
SetThreadState(TS_DebugSuspendPending);
ThreadStore::TrapReturningThreadsIncrement();
ThreadStore::IncrementTrapReturningThreads();
}
}

Expand All @@ -4530,13 +4530,13 @@ inline void Thread::IncrementTraceCallCount()
{
WRAPPER_NO_CONTRACT;
InterlockedIncrement(&m_TraceCallCount);
ThreadStore::TrapReturningThreadsIncrement();
ThreadStore::IncrementTrapReturningThreads();
}

inline void Thread::DecrementTraceCallCount()
{
WRAPPER_NO_CONTRACT;
ThreadStore::TrapReturningThreadsDecrement();
ThreadStore::DecrementTrapReturningThreads();
InterlockedDecrement(&m_TraceCallCount);
}

Expand Down
30 changes: 14 additions & 16 deletions src/coreclr/vm/threadsuspend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1295,7 +1295,7 @@ Thread::UserAbort(EEPolicy::ThreadAbortTypes abortType, DWORD timeout)
// The thread being aborted may clear the TS_AbortRequested bit and the matching increment
// of g_TrapReturningThreads behind our back. Increment g_TrapReturningThreads here
// to ensure that we stop for the stack crawl even if the TS_AbortRequested bit is cleared.
ThreadStore::TrapReturningThreadsIncrement();
ThreadStore::IncrementTrapReturningThreads();
}
void NeedStackCrawl()
{
Expand All @@ -1310,7 +1310,7 @@ Thread::UserAbort(EEPolicy::ThreadAbortTypes abortType, DWORD timeout)
if (m_NeedRelease)
{
m_NeedRelease = FALSE;
ThreadStore::TrapReturningThreadsDecrement();
ThreadStore::DecrementTrapReturningThreads();
ThreadStore::SetStackCrawlEvent();
m_pThread->ResetThreadState(TS_StackCrawlNeeded);
if (!m_fHoldingThreadStoreLock)
Expand Down Expand Up @@ -1755,7 +1755,7 @@ void Thread::SetAbortRequestBit()
}
if (InterlockedCompareExchange((LONG*)&m_State, curValue|TS_AbortRequested, curValue) == curValue)
{
ThreadStore::TrapReturningThreadsIncrement();
ThreadStore::IncrementTrapReturningThreads();

break;
}
Expand All @@ -1771,7 +1771,7 @@ void Thread::RemoveAbortRequestBit()

#ifdef _DEBUG
// There's a race between removing the TS_AbortRequested bit and decrementing g_TrapReturningThreads
// We may remove the bit, but before we have a chance to call ThreadStore::TrapReturningThreadsDecrement()
// We may remove the bit, but before we have a chance to call ThreadStore::DecrementTrapReturningThreads()
// DbgFindThread() may execute, and find too few threads with the bit set.
// To ensure the assert in DbgFindThread does not fire under such a race we set the ChgInFlight before hand.
CounterHolder trtHolder(&g_trtChgInFlight);
Expand All @@ -1785,7 +1785,7 @@ void Thread::RemoveAbortRequestBit()
}
if (InterlockedCompareExchange((LONG*)&m_State, curValue&(~TS_AbortRequested), curValue) == curValue)
{
ThreadStore::TrapReturningThreadsDecrement();
ThreadStore::DecrementTrapReturningThreads();

break;
}
Expand Down Expand Up @@ -2113,7 +2113,7 @@ void Thread::RareDisablePreemptiveGC()
if (ThreadStore::HoldingThreadStore(this))
{
// In theory threads should not try entering coop mode while holding TS lock,
// but some scenarios like GCCoopHackNoThread end up here
// but some scenarios like GCCoopHackNoThread and GCX_COOP_NO_THREAD_BROKEN end up here
goto Exit;
}

Expand Down Expand Up @@ -2143,7 +2143,7 @@ void Thread::RareDisablePreemptiveGC()
{
#ifdef DEBUGGING_SUPPORTED
// If debugger wants the thread to suspend, give the debugger precedence.
if ((m_State & TS_DebugSuspendPending) && !IsInForbidSuspendForDebuggerRegion())
if (HasThreadStateOpportunistic(TS_DebugSuspendPending) && !IsInForbidSuspendForDebuggerRegion())
{
EnablePreemptiveGC();

Expand Down Expand Up @@ -2214,7 +2214,7 @@ void Thread::RareDisablePreemptiveGC()
continue;
}

if (HasThreadState(TS_StackCrawlNeeded))
if (HasThreadStateOpportunistic(TS_StackCrawlNeeded))
{
EnablePreemptiveGC();
ThreadStore::WaitForStackCrawlEvent();
Expand Down Expand Up @@ -2394,7 +2394,7 @@ void Thread::PulseGCMode()
// Indicate whether threads should be trapped when returning to the EE (i.e. disabling
// preemptive GC mode)
Volatile<LONG> g_fTrapReturningThreadsLock;
void ThreadStore::TrapReturningThreadsIncrement()
void ThreadStore::IncrementTrapReturningThreads()
{
CONTRACTL {
NOTHROW;
Expand Down Expand Up @@ -2432,7 +2432,7 @@ void ThreadStore::TrapReturningThreadsIncrement()
g_fTrapReturningThreadsLock = 0;
}

void ThreadStore::TrapReturningThreadsDecrement()
void ThreadStore::DecrementTrapReturningThreads()
{
CONTRACTL {
NOTHROW;
Expand Down Expand Up @@ -3210,14 +3210,14 @@ COR_PRF_SUSPEND_REASON GCSuspendReasonToProfSuspendReason(ThreadSuspend::SUSPEND
}
#endif // PROFILING_SUPPORTED

int64_t QueryPerformanceCounter()
static int64_t QueryPerformanceCounter()
{
LARGE_INTEGER ts;
QueryPerformanceCounter(&ts);
return ts.QuadPart;
}

int64_t QueryPerformanceFrequency()
static int64_t QueryPerformanceFrequency()
{
LARGE_INTEGER ts;
QueryPerformanceFrequency(&ts);
Expand Down Expand Up @@ -3596,8 +3596,6 @@ void EnableStressHeapHelper()
}
#endif

// We're done with our GC. Let all the threads run again.
// By this point we've already unblocked most threads. This just releases the ThreadStore lock.
void ThreadSuspend::ResumeAllThreads(BOOL SuspendSucceeded)
{
CONTRACTL {
Expand Down Expand Up @@ -5359,7 +5357,7 @@ void Thread::MarkForSuspension(ULONG bit)
_ASSERTE((m_State & bit) == 0);

InterlockedOr((LONG*)&m_State, bit);
ThreadStore::TrapReturningThreadsIncrement();
ThreadStore::IncrementTrapReturningThreads();
}

void Thread::UnmarkForSuspension(ULONG mask)
Expand All @@ -5378,7 +5376,7 @@ void Thread::UnmarkForSuspension(ULONG mask)
_ASSERTE((m_State & ~mask) != 0);

// we decrement the global first to be able to satisfy the assert from DbgFindThread
ThreadStore::TrapReturningThreadsDecrement();
ThreadStore::DecrementTrapReturningThreads();
InterlockedAnd((LONG*)&m_State, mask);
}

Expand Down

0 comments on commit a38b54a

Please sign in to comment.