diff --git a/docs/design/datacontracts/Thread.md b/docs/design/datacontracts/Thread.md index 7bee0fe79fdc..2910dc8dbb82 100644 --- a/docs/design/datacontracts/Thread.md +++ b/docs/design/datacontracts/Thread.md @@ -22,9 +22,7 @@ enum ThreadState TS_AbortRequested = 0x00000001, // Abort the thread - TS_GCSuspendPending = 0x00000002, // ThreadSuspend::SuspendRuntime watches this thread to leave coop mode. TS_GCSuspendRedirected = 0x00000004, // ThreadSuspend::SuspendRuntime has redirected the thread to suspention routine. - TS_GCSuspendFlags = TS_GCSuspendPending | TS_GCSuspendRedirected, // used to track suspension progress. Only SuspendRuntime writes/resets these. TS_DebugSuspendPending = 0x00000008, // Is the debugger suspending threads? TS_GCOnTransitions = 0x00000010, // Force a GC on stub transitions (GCStress only) diff --git a/src/coreclr/vm/fcall.cpp b/src/coreclr/vm/fcall.cpp index 20461bef7988..f344018807e7 100644 --- a/src/coreclr/vm/fcall.cpp +++ b/src/coreclr/vm/fcall.cpp @@ -129,7 +129,7 @@ NOINLINE Object* FC_GCPoll(void* __me, Object* objToProtect) INCONTRACT(FCallCheck __fCallCheck(__FILE__, __LINE__)); Thread *thread = GetThread(); - if (thread->CatchAtSafePointOpportunistic()) // Does someone want this thread stopped? + if (thread->CatchAtSafePoint()) // Does someone want this thread stopped? { HELPER_METHOD_FRAME_BEGIN_RET_ATTRIB_1(Frame::FRAME_ATTR_CAPTURE_DEPTH_2, objToProtect); diff --git a/src/coreclr/vm/frames.h b/src/coreclr/vm/frames.h index 512ce37520a7..3d3fe177bcbb 100644 --- a/src/coreclr/vm/frames.h +++ b/src/coreclr/vm/frames.h @@ -1332,7 +1332,7 @@ class HelperMethodFrame : public Frame FORCEINLINE void Poll() { WRAPPER_NO_CONTRACT; - if (m_pThread->CatchAtSafePointOpportunistic()) + if (m_pThread->CatchAtSafePoint()) CommonTripThread(); } #endif // DACCESS_COMPILE diff --git a/src/coreclr/vm/interpreter.cpp b/src/coreclr/vm/interpreter.cpp index 2edb5948d72e..20e42c83eb9b 100644 --- a/src/coreclr/vm/interpreter.cpp +++ b/src/coreclr/vm/interpreter.cpp @@ -1952,7 +1952,7 @@ static void MonitorEnter(Object* obj, BYTE* pbLockTaken) GCPROTECT_BEGININTERIOR(pbLockTaken); - if (GET_THREAD()->CatchAtSafePointOpportunistic()) + if (GET_THREAD()->CatchAtSafePoint()) { GET_THREAD()->PulseGCMode(); } diff --git a/src/coreclr/vm/jithelpers.cpp b/src/coreclr/vm/jithelpers.cpp index 045cef865d78..8304847a675c 100644 --- a/src/coreclr/vm/jithelpers.cpp +++ b/src/coreclr/vm/jithelpers.cpp @@ -3651,7 +3651,7 @@ NOINLINE static void JIT_MonEnter_Helper(Object* obj, BYTE* pbLockTaken, LPVOID GCPROTECT_BEGININTERIOR(pbLockTaken); - if (GET_THREAD()->CatchAtSafePointOpportunistic()) + if (GET_THREAD()->CatchAtSafePoint()) { GET_THREAD()->PulseGCMode(); } @@ -3730,7 +3730,7 @@ NOINLINE static void JIT_MonTryEnter_Helper(Object* obj, INT32 timeOut, BYTE* pb GCPROTECT_BEGININTERIOR(pbLockTaken); - if (GET_THREAD()->CatchAtSafePointOpportunistic()) + if (GET_THREAD()->CatchAtSafePoint()) { GET_THREAD()->PulseGCMode(); } @@ -3764,7 +3764,7 @@ HCIMPL3(void, JIT_MonTryEnter_Portable, Object* obj, INT32 timeOut, BYTE* pbLock pCurThread = GetThread(); - if (pCurThread->CatchAtSafePointOpportunistic()) + if (pCurThread->CatchAtSafePoint()) { goto FramedLockHelper; } @@ -3936,7 +3936,7 @@ HCIMPL_MONHELPER(JIT_MonEnterStatic_Portable, AwareLock *lock) MONHELPER_STATE(_ASSERTE(pbLockTaken != NULL && *pbLockTaken == 0)); Thread *pCurThread = GetThread(); - if (pCurThread->CatchAtSafePointOpportunistic()) + if (pCurThread->CatchAtSafePoint()) { goto FramedLockHelper; } @@ -4830,7 +4830,7 @@ HCIMPL0(VOID, JIT_PollGC) return; // Does someone want this thread stopped? - if (!GetThread()->CatchAtSafePointOpportunistic()) + if (!GetThread()->CatchAtSafePoint()) return; // Tailcall to the slow helper diff --git a/src/coreclr/vm/object.inl b/src/coreclr/vm/object.inl index 491aab1d4c87..c78222529c30 100644 --- a/src/coreclr/vm/object.inl +++ b/src/coreclr/vm/object.inl @@ -114,7 +114,7 @@ FORCEINLINE bool Object::TryEnterObjMonitorSpinHelper() } CONTRACTL_END; Thread *pCurThread = GetThread(); - if (pCurThread->CatchAtSafePointOpportunistic()) + if (pCurThread->CatchAtSafePoint()) { return false; } diff --git a/src/coreclr/vm/syncblk.cpp b/src/coreclr/vm/syncblk.cpp index 606137ee8eba..482b9da36644 100644 --- a/src/coreclr/vm/syncblk.cpp +++ b/src/coreclr/vm/syncblk.cpp @@ -598,7 +598,7 @@ void SyncBlockCache::CleanupSyncBlocks() pParam->psb = NULL; // pulse GC mode to allow GC to perform its work - if (FinalizerThread::GetFinalizerThread()->CatchAtSafePointOpportunistic()) + if (FinalizerThread::GetFinalizerThread()->CatchAtSafePoint()) { FinalizerThread::GetFinalizerThread()->PulseGCMode(); } diff --git a/src/coreclr/vm/threads.cpp b/src/coreclr/vm/threads.cpp index 8a4289f04ff0..d71c4e204365 100644 --- a/src/coreclr/vm/threads.cpp +++ b/src/coreclr/vm/threads.cpp @@ -7065,22 +7065,20 @@ void CommonTripThread() CONTRACTL { THROWS; GC_TRIGGERS; + MODE_COOPERATIVE; } CONTRACTL_END; Thread *thread = GetThread(); - thread->HandleThreadAbort (); + thread->HandleThreadAbort(); - if (thread->CatchAtSafePointOpportunistic()) - { - _ASSERTE(!ThreadStore::HoldingThreadStore(thread)); + _ASSERTE(!ThreadStore::HoldingThreadStore(thread)); #ifdef FEATURE_HIJACK - thread->UnhijackThread(); + thread->UnhijackThread(); #endif // FEATURE_HIJACK - // Trap - thread->PulseGCMode(); - } + // Trap + thread->PulseGCMode(); #else DacNotImpl(); #endif // #ifndef DACCESS_COMPILE diff --git a/src/coreclr/vm/threads.h b/src/coreclr/vm/threads.h index 9f8f08fdaca6..9c6feb5a1de4 100644 --- a/src/coreclr/vm/threads.h +++ b/src/coreclr/vm/threads.h @@ -592,7 +592,7 @@ class Thread public: // If we are trying to suspend a thread, we set the appropriate pending bit to - // indicate why we want to suspend it (TS_GCSuspendPending or TS_DebugSuspendPending). + // indicate why we want to suspend it (TS_AbortRequested or TS_DebugSuspendPending). // // If instead the thread has blocked itself, via WaitSuspendEvent, we indicate // this with TS_SyncSuspended. However, we need to know whether the synchronous @@ -608,9 +608,8 @@ class Thread TS_AbortRequested = 0x00000001, // Abort the thread - TS_GCSuspendPending = 0x00000002, // ThreadSuspend::SuspendRuntime watches this thread to leave coop mode. + // unused = 0x00000002, TS_GCSuspendRedirected = 0x00000004, // ThreadSuspend::SuspendRuntime has redirected the thread to suspention routine. - TS_GCSuspendFlags = TS_GCSuspendPending | TS_GCSuspendRedirected, // used to track suspension progress. Only SuspendRuntime writes/resets these. TS_DebugSuspendPending = 0x00000008, // Is the debugger suspending threads? TS_GCOnTransitions = 0x00000010, // Force a GC on stub transitions (GCStress only) @@ -671,8 +670,7 @@ class Thread // enum is changed, we also need to update SOS to reflect this. // We require (and assert) that the following bits are less than 0x100. - TS_CatchAtSafePoint = (TS_AbortRequested | TS_GCSuspendPending | - TS_DebugSuspendPending | TS_GCOnTransitions), + TS_CatchAtSafePoint = (TS_AbortRequested | TS_DebugSuspendPending | TS_GCOnTransitions), }; // Thread flags that aren't really states in themselves but rather things the thread @@ -937,11 +935,18 @@ class Thread void DoExtraWorkForFinalizer(); #ifndef DACCESS_COMPILE - DWORD CatchAtSafePointOpportunistic() + DWORD CatchAtSafePoint() { - LIMITED_METHOD_CONTRACT; + CONTRACTL + { + NOTHROW; + GC_NOTRIGGER; + MODE_COOPERATIVE; + } + CONTRACTL_END; - return HasThreadStateOpportunistic(TS_CatchAtSafePoint); + return g_TrapReturningThreads & 1 || + HasThreadStateOpportunistic(TS_CatchAtSafePoint); } #endif // DACCESS_COMPILE @@ -3222,8 +3227,6 @@ class Thread if (SuspendSucceeded) UnhijackThread(); #endif // FEATURE_HIJACK - - _ASSERTE(!HasThreadStateOpportunistic(Thread::TS_GCSuspendPending)); } static LPVOID GetStaticFieldAddress(FieldDesc *pFD); diff --git a/src/coreclr/vm/threadsuspend.cpp b/src/coreclr/vm/threadsuspend.cpp index 136016cd607f..e513f8fb1903 100644 --- a/src/coreclr/vm/threadsuspend.cpp +++ b/src/coreclr/vm/threadsuspend.cpp @@ -3317,7 +3317,6 @@ void ThreadSuspend::SuspendAllThreads() // This thread Thread *pCurThread = GetThreadNULLOk(); - // // Remember that we're the one suspending the EE // @@ -3333,8 +3332,6 @@ void ThreadSuspend::SuspendAllThreads() // ThreadStore::SetThreadTrapForSuspension(); - _ASSERTE(!pCurThread || !pCurThread->HasThreadState(Thread::TS_GCSuspendFlags)); - // Flush the store buffers on all CPUs, to ensure two things: // - we get a reliable reading of the threads' m_fPreemptiveGCDisabled state // - other threads see that g_TrapReturningThreads is set @@ -3359,24 +3356,12 @@ void ThreadSuspend::SuspendAllThreads() if (pTargetThread->m_fPreemptiveGCDisabled.LoadWithoutBarrier()) { - if (!pTargetThread->HasThreadStateOpportunistic(Thread::TS_GCSuspendPending)) - { - pTargetThread->SetThreadState(Thread::TS_GCSuspendPending); - } - remaining++; if (!observeOnly) { pTargetThread->Hijack(); } } - else - { - if (pTargetThread->HasThreadStateOpportunistic(Thread::TS_GCSuspendPending)) - { - pTargetThread->ResetThreadState(Thread::TS_GCSuspendPending); - } - } } if (!remaining) @@ -3503,12 +3488,10 @@ void Thread::Hijack() } // the thread is suspended here, we can hijack, if it is stopped in hijackable location - if (!m_fPreemptiveGCDisabled.LoadWithoutBarrier()) { // actually, we are done with this one STRESS_LOG1(LF_SYNC, LL_INFO1000, " Thread %x went preemptive while suspending it is at a GC safe point\n", this); - ResetThreadState(Thread::TS_GCSuspendFlags); ResumeThread(); return; } @@ -5662,7 +5645,6 @@ void ThreadSuspend::SuspendEE(SUSPEND_REASON reason) while ((thread = ThreadStore::GetThreadList(thread)) != NULL) { thread->DisableStressHeap(); - _ASSERTE(!thread->HasThreadState(Thread::TS_GCSuspendPending)); } } #endif @@ -5717,7 +5699,7 @@ void ThreadSuspend::SuspendEE(SUSPEND_REASON reason) LOG((LF_GCROOTS | LF_GC | LF_CORDB, LL_INFO10, "The EE is free now...\n")); // If someone's trying to suspend *this* thread, this is a good opportunity. - if (pCurThread && pCurThread->CatchAtSafePointOpportunistic()) + if (pCurThread && pCurThread->CatchAtSafePoint()) { pCurThread->PulseGCMode(); // Go suspend myself. }