From eb84def758d32fb7ee44f488b29911a186985cf0 Mon Sep 17 00:00:00 2001 From: Koundinya Veluri Date: Mon, 27 Mar 2017 11:10:07 -0700 Subject: [PATCH] =?utf8?q?Add=20heuristic=20to=20trigger=20GC=20to=20final?= =?utf8?q?ize=20dead=20threads=20and=20clean=20up=20han=E2=80=A6=20(dotnet?= =?utf8?q?/coreclr#10413)?= MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Add heuristic to trigger GC to finalize dead threads and clean up handles and memory A thread that is started allocates some handles in Thread::AllocHandles(). After it terminates, the managed thread object needs to be collected by a GC for the handles to be released. Some applications that are mostly idle but have a timer ticking periodically will cause a thread pool thread to be created on each tick, since the thread pool preserves threads for a maximum of 20 seconds currently. Over time the number of handles accumulates to a high value. Thread creation adds some memory pressure, but that does not force a GC until a sufficiently large handle count, and for some mostly idle apps, that can be very long. The same issue arises with directly starting threads as well. Fixes dotnet/coreclr#6602: - Track a dead thread count separately from the current dead thread count. This is the count that will contribute to triggering a GC, once it reaches a threshold. The count is tracked separately so that it may be reset to zero when a max-generation GC occurs, preventing dead threads that survive a GC due to references from contributing to triggering a GC again in this fashion. - If the count exceeds a threshold, enumerate dead threads to see which GC generation the corresponding managed thread objects are in. If the duration of time since the last GC of the desired generation also exceeds a threshold, trigger a preferably non-blocking GC on the finalizer thread. - Removed a couple of handles and some code paths specific to user-requested thread suspension, which is not supported on CoreCLR Commit migrated from https://github.com/dotnet/coreclr/commit/2401b6ed08252d48831bfd804c3533cd0142c76c --- src/coreclr/src/debug/daccess/dacdbiimpl.cpp | 11 +- src/coreclr/src/inc/clrconfigvalues.h | 6 + src/coreclr/src/vm/comsynchronizable.cpp | 14 +- src/coreclr/src/vm/debugdebugger.cpp | 58 +---- src/coreclr/src/vm/eedbginterfaceimpl.cpp | 11 +- src/coreclr/src/vm/finalizerthread.cpp | 2 + src/coreclr/src/vm/gcenv.ee.cpp | 5 + src/coreclr/src/vm/threads.cpp | 257 +++++++++++++++++---- src/coreclr/src/vm/threads.h | 60 ++++- src/coreclr/src/vm/threadsuspend.cpp | 69 ++---- .../threading/DeadThreads/DeadThreads.cs | 43 ++++ .../threading/DeadThreads/DeadThreads.csproj | 32 +++ 12 files changed, 374 insertions(+), 194 deletions(-) create mode 100644 src/coreclr/tests/src/baseservices/threading/DeadThreads/DeadThreads.cs create mode 100644 src/coreclr/tests/src/baseservices/threading/DeadThreads/DeadThreads.csproj diff --git a/src/coreclr/src/debug/daccess/dacdbiimpl.cpp b/src/coreclr/src/debug/daccess/dacdbiimpl.cpp index c7595c4..74ba14f 100644 --- a/src/coreclr/src/debug/daccess/dacdbiimpl.cpp +++ b/src/coreclr/src/debug/daccess/dacdbiimpl.cpp @@ -5452,15 +5452,8 @@ CorDebugUserState DacDbiInterfaceImpl::GetPartialUserState(VMPTR_Thread vmThread result |= USER_WAIT_SLEEP_JOIN; } - // Don't report a SuspendRequested if the thread has actually Suspended. - if ((ts & Thread::TS_UserSuspendPending) && (ts & Thread::TS_SyncSuspended)) - { - result |= USER_SUSPENDED; - } - else if (ts & Thread::TS_UserSuspendPending) - { - result |= USER_SUSPEND_REQUESTED; - } + // CoreCLR does not support user-requested thread suspension + _ASSERTE(!(ts & Thread::TS_UserSuspendPending)); if (pThread->IsThreadPoolThread()) { diff --git a/src/coreclr/src/inc/clrconfigvalues.h b/src/coreclr/src/inc/clrconfigvalues.h index 6160f20..b24ef39 100644 --- a/src/coreclr/src/inc/clrconfigvalues.h +++ b/src/coreclr/src/inc/clrconfigvalues.h @@ -936,6 +936,12 @@ CONFIG_DWORD_INFO(INTERNAL_SuspendDeadlockTimeout, W("SuspendDeadlockTimeout"), CONFIG_DWORD_INFO(INTERNAL_SuspendThreadDeadlockTimeoutMs, W("SuspendThreadDeadlockTimeoutMs"), 2000, "") RETAIL_CONFIG_DWORD_INFO(INTERNAL_ThreadSuspendInjection, W("INTERNAL_ThreadSuspendInjection"), 1, "Specifies whether to inject activations for thread suspension on Unix") +// +// Thread (miscellaneous) +// +RETAIL_CONFIG_DWORD_INFO(INTERNAL_Thread_DeadThreadCountThresholdForGCTrigger, W("Thread_DeadThreadCountThresholdForGCTrigger"), 75, "In the heuristics to clean up dead threads, this threshold must be reached before triggering a GC will be considered. Set to 0 to disable triggering a GC based on dead threads.") +RETAIL_CONFIG_DWORD_INFO(INTERNAL_Thread_DeadThreadGCTriggerPeriodMilliseconds, W("Thread_DeadThreadGCTriggerPeriodMilliseconds"), 1000 * 60 * 30, "In the heuristics to clean up dead threads, this much time must have elapsed since the previous max-generation GC before triggering another GC will be considered") + // // Threadpool // diff --git a/src/coreclr/src/vm/comsynchronizable.cpp b/src/coreclr/src/vm/comsynchronizable.cpp index b8f871d..08b5281 100644 --- a/src/coreclr/src/vm/comsynchronizable.cpp +++ b/src/coreclr/src/vm/comsynchronizable.cpp @@ -926,18 +926,8 @@ FCIMPL1(INT32, ThreadNative::GetThreadState, ThreadBaseObject* pThisUNSAFE) if (state & Thread::TS_Interruptible) res |= ThreadWaitSleepJoin; - // Don't report a SuspendRequested if the thread has actually Suspended. - if ((state & Thread::TS_UserSuspendPending) && - (state & Thread::TS_SyncSuspended) - ) - { - res |= ThreadSuspended; - } - else - if (state & Thread::TS_UserSuspendPending) - { - res |= ThreadSuspendRequested; - } + // CoreCLR does not support user-requested thread suspension + _ASSERTE(!(state & Thread::TS_UserSuspendPending)); HELPER_METHOD_POLL(); HELPER_METHOD_FRAME_END(); diff --git a/src/coreclr/src/vm/debugdebugger.cpp b/src/coreclr/src/vm/debugdebugger.cpp index 32c6cd6..489c623 100644 --- a/src/coreclr/src/vm/debugdebugger.cpp +++ b/src/coreclr/src/vm/debugdebugger.cpp @@ -937,62 +937,8 @@ void DebugStackTrace::GetStackFramesHelper(Frame *pStartFrame, goto LSafeToTrace; } - if (state & Thread::TS_UserSuspendPending) - { - if (state & Thread::TS_SyncSuspended) - { - goto LSafeToTrace; - } - -#ifndef DISABLE_THREADSUSPEND - // On Mac don't perform the optimization below, but rather wait for - // the suspendee to set the TS_SyncSuspended flag - - // The target thread is not actually suspended yet, but if it is - // in preemptive mode, then it is still safe to trace. Before we - // can look at another thread's GC mode, we have to suspend it: - // The target thread updates its GC mode flag with non-interlocked - // operations, and Thread::SuspendThread drains the CPU's store - // buffer (by virtue of calling GetThreadContext). - switch (pThread->SuspendThread()) - { - case Thread::STR_Success: - if (!pThread->PreemptiveGCDisabledOther()) - { - pThread->ResumeThread(); - goto LSafeToTrace; - } - - // Refuse to trace the stack. - // - // Note that there is a pretty large window in-between when the - // target thread sets the GC mode to cooperative, and when it - // actually sets the TS_SyncSuspended bit. In this window, we - // will refuse to take a stack trace even though it would be - // safe to do so. - pThread->ResumeThread(); - break; - case Thread::STR_Failure: - case Thread::STR_NoStressLog: - break; - case Thread::STR_UnstartedOrDead: - // We know the thread is not unstarted, because we checked for - // TS_Unstarted above. - _ASSERTE(!(state & Thread::TS_Unstarted)); - - // Since the thread is dead, it is safe to trace. - goto LSafeToTrace; - case Thread::STR_SwitchedOut: - if (!pThread->PreemptiveGCDisabledOther()) - { - goto LSafeToTrace; - } - break; - default: - UNREACHABLE(); - } -#endif // DISABLE_THREADSUSPEND - } + // CoreCLR does not support user-requested thread suspension + _ASSERTE(!(state & Thread::TS_UserSuspendPending)); COMPlusThrow(kThreadStateException, IDS_EE_THREAD_BAD_STATE); diff --git a/src/coreclr/src/vm/eedbginterfaceimpl.cpp b/src/coreclr/src/vm/eedbginterfaceimpl.cpp index b7a4359..ede82c7 100644 --- a/src/coreclr/src/vm/eedbginterfaceimpl.cpp +++ b/src/coreclr/src/vm/eedbginterfaceimpl.cpp @@ -1588,15 +1588,8 @@ CorDebugUserState EEDbgInterfaceImpl::GetPartialUserState(Thread *pThread) ret |= (unsigned)USER_WAIT_SLEEP_JOIN; } - // Don't report a SuspendRequested if the thread has actually Suspended. - if ( ((ts & Thread::TS_UserSuspendPending) && (ts & Thread::TS_SyncSuspended))) - { - ret |= (unsigned)USER_SUSPENDED; - } - else if (ts & Thread::TS_UserSuspendPending) - { - ret |= (unsigned)USER_SUSPEND_REQUESTED; - } + // CoreCLR does not support user-requested thread suspension + _ASSERTE(!(ts & Thread::TS_UserSuspendPending)); LOG((LF_CORDB,LL_INFO1000, "EEDbgII::GUS: thread 0x%x (id:0x%x)" " userThreadState is 0x%x\n", pThread, pThread->GetThreadId(), ret)); diff --git a/src/coreclr/src/vm/finalizerthread.cpp b/src/coreclr/src/vm/finalizerthread.cpp index 51a2ed4..d111eeb 100644 --- a/src/coreclr/src/vm/finalizerthread.cpp +++ b/src/coreclr/src/vm/finalizerthread.cpp @@ -1345,6 +1345,8 @@ BOOL FinalizerThread::FinalizerThreadWatchDogHelper() } ULONGLONG dwCurTickCount = CLRGetTickCount64(); if (pThread && pThread->m_State & (Thread::TS_UserSuspendPending | Thread::TS_DebugSuspendPending)) { + // CoreCLR does not support user-requested thread suspension + _ASSERTE(!(pThread->m_State & Thread::TS_UserSuspendPending)); dwBeginTickCount = dwCurTickCount; } if (dwCurTickCount - dwBeginTickCount >= maxTotalWait) diff --git a/src/coreclr/src/vm/gcenv.ee.cpp b/src/coreclr/src/vm/gcenv.ee.cpp index bf2ce2c..392c179 100644 --- a/src/coreclr/src/vm/gcenv.ee.cpp +++ b/src/coreclr/src/vm/gcenv.ee.cpp @@ -626,6 +626,11 @@ void GCToEEInterface::GcStartWork (int condemned, int max_gen) // RCWWalker::OnGCStarted(condemned); #endif // FEATURE_COMINTEROP + + if (condemned == max_gen) + { + ThreadStore::s_pThreadStore->OnMaxGenerationGCStarted(); + } } void GCToEEInterface::GcDone(int condemned) diff --git a/src/coreclr/src/vm/threads.cpp b/src/coreclr/src/vm/threads.cpp index b5f7507..6d9d10b 100644 --- a/src/coreclr/src/vm/threads.cpp +++ b/src/coreclr/src/vm/threads.cpp @@ -1825,6 +1825,7 @@ Thread::Thread() #ifdef FEATURE_COMINTEROP m_fDisableComObjectEagerCleanup = false; #endif //FEATURE_COMINTEROP + m_fHasDeadThreadBeenConsideredForGCTrigger = false; m_Context = NULL; m_TraceCallCount = 0; m_ThrewControlForThread = 0; @@ -2170,28 +2171,17 @@ BOOL Thread::AllocHandles() { WRAPPER_NO_CONTRACT; - _ASSERTE(!m_SafeEvent.IsValid()); - _ASSERTE(!m_UserSuspendEvent.IsValid()); _ASSERTE(!m_DebugSuspendEvent.IsValid()); _ASSERTE(!m_EventWait.IsValid()); BOOL fOK = TRUE; EX_TRY { // create a manual reset event for getting the thread to a safe point - m_SafeEvent.CreateManualEvent(FALSE); - m_UserSuspendEvent.CreateManualEvent(FALSE); m_DebugSuspendEvent.CreateManualEvent(FALSE); m_EventWait.CreateManualEvent(TRUE); } EX_CATCH { fOK = FALSE; - if (!m_SafeEvent.IsValid()) { - m_SafeEvent.CloseEvent(); - } - - if (!m_UserSuspendEvent.IsValid()) { - m_UserSuspendEvent.CloseEvent(); - } if (!m_DebugSuspendEvent.IsValid()) { m_DebugSuspendEvent.CloseEvent(); @@ -2404,30 +2394,8 @@ FAILURE: } #endif // PROFILING_SUPPORTED - // Is there a pending user suspension? - if (m_State & TS_SuspendUnstarted) - { - BOOL doSuspend = FALSE; - - { - ThreadStoreLockHolder TSLockHolder; - - // Perhaps we got resumed before it took effect? - if (m_State & TS_SuspendUnstarted) - { - FastInterlockAnd((ULONG *) &m_State, ~TS_SuspendUnstarted); - SetupForSuspension(TS_UserSuspendPending); - MarkForSuspension(TS_UserSuspendPending); - doSuspend = TRUE; - } - } - - if (doSuspend) - { - GCX_PREEMP(); - WaitSuspendEvents(); - } - } + // CoreCLR does not support user-requested thread suspension + _ASSERTE(!(m_State & TS_SuspendUnstarted)); } return res; @@ -3064,14 +3032,6 @@ Thread::~Thread() CloseHandle(GetThreadHandle()); } - if (m_SafeEvent.IsValid()) - { - m_SafeEvent.CloseEvent(); - } - if (m_UserSuspendEvent.IsValid()) - { - m_UserSuspendEvent.CloseEvent(); - } if (m_DebugSuspendEvent.IsValid()) { m_DebugSuspendEvent.CloseEvent(); @@ -3503,6 +3463,7 @@ void Thread::OnThreadTerminate(BOOL holdingLock) FastInterlockOr((ULONG *) &m_State, TS_Dead); ThreadStore::s_pThreadStore->m_DeadThreadCount++; + ThreadStore::s_pThreadStore->IncrementDeadThreadCountForGCTrigger(); if (IsUnstarted()) ThreadStore::s_pThreadStore->m_UnstartedThreadCount--; @@ -3527,8 +3488,8 @@ void Thread::OnThreadTerminate(BOOL holdingLock) if (m_State & TS_DebugSuspendPending) UnmarkForSuspension(~TS_DebugSuspendPending); - if (m_State & TS_UserSuspendPending) - UnmarkForSuspension(~TS_UserSuspendPending); + // CoreCLR does not support user-requested thread suspension + _ASSERTE(!(m_State & TS_UserSuspendPending)); if (CurrentThreadID == ThisThreadID && IsAbortRequested()) { @@ -5739,6 +5700,8 @@ ThreadStore::ThreadStore() m_BackgroundThreadCount(0), m_PendingThreadCount(0), m_DeadThreadCount(0), + m_DeadThreadCountForGCTrigger(0), + m_TriggerGCForDeadThreads(false), m_GuidCreated(FALSE), m_HoldingThread(0) { @@ -5778,6 +5741,15 @@ void ThreadStore::InitThreadStore() s_pWaitForStackCrawlEvent = new CLREvent(); s_pWaitForStackCrawlEvent->CreateManualEvent(FALSE); + + s_DeadThreadCountThresholdForGCTrigger = + static_cast(CLRConfig::GetConfigValue(CLRConfig::INTERNAL_Thread_DeadThreadCountThresholdForGCTrigger)); + if (s_DeadThreadCountThresholdForGCTrigger < 0) + { + s_DeadThreadCountThresholdForGCTrigger = 0; + } + s_DeadThreadGCTriggerPeriodMilliseconds = + CLRConfig::GetConfigValue(CLRConfig::INTERNAL_Thread_DeadThreadGCTriggerPeriodMilliseconds); } // Enter and leave the critical section around the thread store. Clients should @@ -5920,7 +5892,10 @@ BOOL ThreadStore::RemoveThread(Thread *target) s_pThreadStore->m_ThreadCount--; if (target->IsDead()) + { s_pThreadStore->m_DeadThreadCount--; + s_pThreadStore->DecrementDeadThreadCountForGCTrigger(); + } // Unstarted threads are not in the Background count: if (target->IsUnstarted()) @@ -6009,6 +5984,188 @@ void ThreadStore::TransferStartedThread(Thread *thread, BOOL bRequiresTSL) CheckForEEShutdown(); } +LONG ThreadStore::s_DeadThreadCountThresholdForGCTrigger = 0; +DWORD ThreadStore::s_DeadThreadGCTriggerPeriodMilliseconds = 0; + +void ThreadStore::IncrementDeadThreadCountForGCTrigger() +{ + CONTRACTL { + NOTHROW; + GC_NOTRIGGER; + } + CONTRACTL_END; + + // Although all increments and decrements are usually done inside a lock, that is not sufficient to synchronize with a + // background GC thread resetting this value, hence the interlocked operation. Ignore overflow; overflow would likely never + // occur, the count is treated as unsigned, and nothing bad would happen if it were to overflow. + SIZE_T count = static_cast(FastInterlockIncrement(&m_DeadThreadCountForGCTrigger)); + + SIZE_T countThreshold = static_cast(s_DeadThreadCountThresholdForGCTrigger); + if (count < countThreshold || countThreshold == 0) + { + return; + } + + IGCHeap *gcHeap = GCHeapUtilities::GetGCHeap(); + if (gcHeap == nullptr) + { + return; + } + + SIZE_T gcLastMilliseconds = gcHeap->GetLastGCStartTime(max_generation); + SIZE_T gcNowMilliseconds = gcHeap->GetNow(); + if (gcNowMilliseconds - gcLastMilliseconds < s_DeadThreadGCTriggerPeriodMilliseconds) + { + return; + } + + if (!g_fEEStarted) // required for FinalizerThread::EnableFinalization() below + { + return; + } + + // The GC is triggered on the finalizer thread since it's not safe to trigger it on DLL_THREAD_DETACH. + // TriggerGCForDeadThreadsIfNecessary() will determine which generation of GC to trigger, and may not actually trigger a GC. + // If a GC is triggered, since there would be a delay before the dead thread count is updated, clear the count and wait for + // it to reach the threshold again. If a GC would not be triggered, the count is still cleared here to prevent waking up the + // finalizer thread to do the work in TriggerGCForDeadThreadsIfNecessary() for every dead thread. + m_DeadThreadCountForGCTrigger = 0; + m_TriggerGCForDeadThreads = true; + FinalizerThread::EnableFinalization(); +} + +void ThreadStore::DecrementDeadThreadCountForGCTrigger() +{ + CONTRACTL { + NOTHROW; + GC_NOTRIGGER; + } + CONTRACTL_END; + + // Although all increments and decrements are usually done inside a lock, that is not sufficient to synchronize with a + // background GC thread resetting this value, hence the interlocked operation. + if (FastInterlockDecrement(&m_DeadThreadCountForGCTrigger) < 0) + { + m_DeadThreadCountForGCTrigger = 0; + } +} + +void ThreadStore::OnMaxGenerationGCStarted() +{ + LIMITED_METHOD_CONTRACT; + + // A dead thread may contribute to triggering a GC at most once. After a max-generation GC occurs, if some dead thread + // objects are still reachable due to references to the thread objects, they will not contribute to triggering a GC again. + // Synchronize the store with increment/decrement operations occurring on different threads, and make the change visible to + // other threads in order to prevent unnecessary GC triggers. + FastInterlockExchange(&m_DeadThreadCountForGCTrigger, 0); +} + +bool ThreadStore::ShouldTriggerGCForDeadThreads() +{ + LIMITED_METHOD_CONTRACT; + + return m_TriggerGCForDeadThreads; +} + +void ThreadStore::TriggerGCForDeadThreadsIfNecessary() +{ + CONTRACTL { + NOTHROW; + GC_TRIGGERS; + } + CONTRACTL_END; + + if (!m_TriggerGCForDeadThreads) + { + return; + } + m_TriggerGCForDeadThreads = false; + + if (g_fEEShutDown) + { + // Not safe to touch CLR state + return; + } + + int gcGenerationToTrigger = 0; + IGCHeap *gcHeap = GCHeapUtilities::GetGCHeap(); + _ASSERTE(gcHeap != nullptr); + SIZE_T generationCountThreshold = static_cast(s_DeadThreadCountThresholdForGCTrigger) / 2; + SIZE_T newDeadThreadGenerationCounts[max_generation + 1] = {0}; + { + ThreadStoreLockHolder threadStoreLockHolder; + GCX_COOP(); + + // Determine the generation for which to trigger a GC. Iterate over all dead threads that have not yet been considered + // for triggering a GC and see how many are in which generations. + for (Thread *thread = ThreadStore::GetAllThreadList(NULL, Thread::TS_Dead, Thread::TS_Dead); + thread != nullptr; + thread = ThreadStore::GetAllThreadList(thread, Thread::TS_Dead, Thread::TS_Dead)) + { + if (thread->HasDeadThreadBeenConsideredForGCTrigger()) + { + continue; + } + + Object *exposedObject = OBJECTREFToObject(thread->GetExposedObjectRaw()); + if (exposedObject == nullptr) + { + continue; + } + + int exposedObjectGeneration = gcHeap->WhichGeneration(exposedObject); + SIZE_T newDeadThreadGenerationCount = ++newDeadThreadGenerationCounts[exposedObjectGeneration]; + if (exposedObjectGeneration > gcGenerationToTrigger && newDeadThreadGenerationCount >= generationCountThreshold) + { + gcGenerationToTrigger = exposedObjectGeneration; + if (gcGenerationToTrigger >= max_generation) + { + break; + } + } + } + + // Make sure that enough time has elapsed since the last GC of the desired generation. We don't want to trigger GCs + // based on this heuristic too often. Give it some time to let the memory pressure trigger GCs automatically, and only + // if it doesn't in the given time, this heuristic may kick in to trigger a GC. + SIZE_T gcLastMilliseconds = gcHeap->GetLastGCStartTime(gcGenerationToTrigger); + SIZE_T gcNowMilliseconds = gcHeap->GetNow(); + if (gcNowMilliseconds - gcLastMilliseconds < s_DeadThreadGCTriggerPeriodMilliseconds) + { + return; + } + + // For threads whose exposed objects are in the generation of GC that will be triggered or in a lower GC generation, + // mark them as having contributed to a GC trigger to prevent redundant GC triggers + for (Thread *thread = ThreadStore::GetAllThreadList(NULL, Thread::TS_Dead, Thread::TS_Dead); + thread != nullptr; + thread = ThreadStore::GetAllThreadList(thread, Thread::TS_Dead, Thread::TS_Dead)) + { + if (thread->HasDeadThreadBeenConsideredForGCTrigger()) + { + continue; + } + + Object *exposedObject = OBJECTREFToObject(thread->GetExposedObjectRaw()); + if (exposedObject == nullptr) + { + continue; + } + + if (gcGenerationToTrigger < max_generation && + static_cast(gcHeap->WhichGeneration(exposedObject)) > gcGenerationToTrigger) + { + continue; + } + + thread->SetHasDeadThreadBeenConsideredForGCTrigger(); + } + } // ThreadStoreLockHolder, GCX_COOP() + + GCHeapUtilities::GetGCHeap()->GarbageCollect(gcGenerationToTrigger, FALSE, collection_non_blocking); +} + #endif // #ifndef DACCESS_COMPILE @@ -6226,8 +6383,8 @@ Retry: if (cur->m_State & Thread::TS_DebugSuspendPending) cntReturn++; - if (cur->m_State & Thread::TS_UserSuspendPending) - cntReturn++; + // CoreCLR does not support user-requested thread suspension + _ASSERTE(!(cur->m_State & Thread::TS_UserSuspendPending)); if (cur->m_TraceCallCount > 0) cntReturn++; @@ -8697,7 +8854,8 @@ BOOL Thread::HaveExtraWorkForFinalizer() || Thread::CleanupNeededForFinalizedThread() || (m_DetachCount > 0) || AppDomain::HasWorkForFinalizerThread() - || SystemDomain::System()->RequireAppDomainCleanup(); + || SystemDomain::System()->RequireAppDomainCleanup() + || ThreadStore::s_pThreadStore->ShouldTriggerGCForDeadThreads(); } void Thread::DoExtraWorkForFinalizer() @@ -8754,7 +8912,8 @@ void Thread::DoExtraWorkForFinalizer() // If there were any TimerInfos waiting to be released, they'll get flushed now ThreadpoolMgr::FlushQueueOfTimerInfos(); - + + ThreadStore::s_pThreadStore->TriggerGCForDeadThreadsIfNecessary(); } diff --git a/src/coreclr/src/vm/threads.h b/src/coreclr/src/vm/threads.h index ff0d166..f34066f 100644 --- a/src/coreclr/src/vm/threads.h +++ b/src/coreclr/src/vm/threads.h @@ -1450,6 +1450,24 @@ public: } #endif //FEATURE_COMINTEROP +#ifndef DACCESS_COMPILE + bool HasDeadThreadBeenConsideredForGCTrigger() + { + LIMITED_METHOD_CONTRACT; + _ASSERTE(IsDead()); + + return m_fHasDeadThreadBeenConsideredForGCTrigger; + } + + void SetHasDeadThreadBeenConsideredForGCTrigger() + { + LIMITED_METHOD_CONTRACT; + _ASSERTE(IsDead()); + + m_fHasDeadThreadBeenConsideredForGCTrigger = true; + } +#endif // !DACCESS_COMPILE + // returns if there is some extra work for the finalizer thread. BOOL HaveExtraWorkForFinalizer(); @@ -3712,9 +3730,11 @@ private: void SetupForSuspension(ULONG bit) { WRAPPER_NO_CONTRACT; - if (bit & TS_UserSuspendPending) { - m_UserSuspendEvent.Reset(); - } + + // CoreCLR does not support user-requested thread suspension + _ASSERTE(!(bit & TS_UserSuspendPending)); + + if (bit & TS_DebugSuspendPending) { m_DebugSuspendEvent.Reset(); } @@ -3731,8 +3751,14 @@ private: // ThreadState oldState = m_State; + // CoreCLR does not support user-requested thread suspension + _ASSERTE(!(oldState & TS_UserSuspendPending)); + while ((oldState & (TS_UserSuspendPending | TS_DebugSuspendPending)) == 0) { + // CoreCLR does not support user-requested thread suspension + _ASSERTE(!(oldState & TS_UserSuspendPending)); + // // Construct the destination state we desire - all suspension bits turned off. // @@ -3751,9 +3777,8 @@ private: oldState = m_State; } - if (bit & TS_UserSuspendPending) { - m_UserSuspendEvent.Set(); - } + // CoreCLR does not support user-requested thread suspension + _ASSERTE(!(bit & TS_UserSuspendPending)); if (bit & TS_DebugSuspendPending) { m_DebugSuspendEvent.Set(); @@ -3761,10 +3786,6 @@ private: } - // For getting a thread to a safe point. A client waits on the event, which is - // set by the thread when it reaches a safe spot. - void SetSafeEvent(); - public: FORCEINLINE void UnhijackThreadNoAlloc() { @@ -3885,8 +3906,6 @@ public: private: // For suspends: - CLREvent m_SafeEvent; - CLREvent m_UserSuspendEvent; CLREvent m_DebugSuspendEvent; // For Object::Wait, Notify and NotifyAll, we use an Event inside the @@ -5230,6 +5249,9 @@ private: // Disables pumping and thread join in RCW creation bool m_fDisableComObjectEagerCleanup; + // See ThreadStore::TriggerGCForDeadThreadsIfNecessary() + bool m_fHasDeadThreadBeenConsideredForGCTrigger; + private: CLRRandom m_random; @@ -5516,6 +5538,8 @@ private: LONG m_PendingThreadCount; LONG m_DeadThreadCount; + LONG m_DeadThreadCountForGCTrigger; + bool m_TriggerGCForDeadThreads; private: // Space for the lazily-created GUID. @@ -5528,6 +5552,10 @@ private: Thread *m_HoldingThread; EEThreadId m_holderthreadid; // current holder (or NULL) +private: + static LONG s_DeadThreadCountThresholdForGCTrigger; + static DWORD s_DeadThreadGCTriggerPeriodMilliseconds; + public: static BOOL HoldingThreadStore() @@ -5601,6 +5629,14 @@ public: LIMITED_METHOD_CONTRACT; s_pWaitForStackCrawlEvent->Reset(); } + +private: + void IncrementDeadThreadCountForGCTrigger(); + void DecrementDeadThreadCountForGCTrigger(); +public: + void OnMaxGenerationGCStarted(); + bool ShouldTriggerGCForDeadThreads(); + void TriggerGCForDeadThreadsIfNecessary(); }; struct TSSuspendHelper { diff --git a/src/coreclr/src/vm/threadsuspend.cpp b/src/coreclr/src/vm/threadsuspend.cpp index 10032fd..71409ce 100644 --- a/src/coreclr/src/vm/threadsuspend.cpp +++ b/src/coreclr/src/vm/threadsuspend.cpp @@ -1978,6 +1978,9 @@ LRetry: m_dwAbortPoint = 7; #endif + // CoreCLR does not support user-requested thread suspension + _ASSERTE(!(m_State & TS_UserSuspendPending)); + // // If it's stopped by the debugger, we don't want to throw an exception. // Debugger suspension is to have no effect of the runtime behaviour. @@ -3090,6 +3093,9 @@ void Thread::RareDisablePreemptiveGC() goto Exit; } + // CoreCLR does not support user-requested thread suspension + _ASSERTE(!(m_State & TS_UserSuspendPending)); + // Note IsGCInProgress is also true for say Pause (anywhere SuspendEE happens) and GCThread is the // thread that did the Pause. While in Pause if another thread attempts Rev/Pinvoke it should get inside the following and // block until resume @@ -3105,6 +3111,9 @@ void Thread::RareDisablePreemptiveGC() do { + // CoreCLR does not support user-requested thread suspension + _ASSERTE(!(m_State & TS_UserSuspendPending)); + EnablePreemptiveGC(); // Cannot use GCX_PREEMP_NO_DTOR here because we're inside of the thread @@ -3558,7 +3567,6 @@ void Thread::RareEnablePreemptiveGC() #endif // FEATURE_HIJACK // wake up any threads waiting to suspend us, like the GC thread. - SetSafeEvent(); ThreadSuspend::g_pGCSuspendEvent->Set(); // for GC, the fact that we are leaving the EE means that it no longer needs to @@ -3566,6 +3574,9 @@ void Thread::RareEnablePreemptiveGC() // Give the debugger precedence over user suspensions: while (m_State & (TS_DebugSuspendPending | TS_UserSuspendPending)) { + // CoreCLR does not support user-requested thread suspension + _ASSERTE(!(m_State & TS_UserSuspendPending)); + #ifdef DEBUGGING_SUPPORTED // We don't notify the debugger that this thread is now suspended. We'll just // let the debugger's helper thread sweep and pick it up. @@ -3903,13 +3914,6 @@ void __stdcall Thread::RedirectedHandledJITCase(RedirectReason reason) // Enable PGC before calling out to the client to allow runtime suspend to finish GCX_PREEMP_NO_DTOR(); - // @TODO: Is this necessary? Does debugger wait on the events, or does it just - // poll every so often? - // Notify the thread that is performing the suspension that this thread - // is now in PGC mode and that it can remove this thread from the list of - // threads it needs to wait for. - pThread->SetSafeEvent(); - // Notify the interface of the pending suspension switch (reason) { case RedirectReason_GCSuspension: @@ -6016,20 +6020,6 @@ void Thread::SysResumeFromDebug(AppDomain *pAppDomain) LOG((LF_CORDB, LL_INFO1000, "RESUME: resume complete. Trap count: %d\n", g_TrapReturningThreads.Load())); } - - -void Thread::SetSafeEvent() -{ - CONTRACTL { - NOTHROW; - GC_NOTRIGGER; - } - CONTRACTL_END; - - m_SafeEvent.Set(); -} - - /* * * WaitSuspendEventsHelper @@ -6056,28 +6046,10 @@ BOOL Thread::WaitSuspendEventsHelper(void) EX_TRY { - if (m_State & TS_UserSuspendPending) { + // CoreCLR does not support user-requested thread suspension + _ASSERTE(!(m_State & TS_UserSuspendPending)); - ThreadState oldState = m_State; - - while (oldState & TS_UserSuspendPending) { - - ThreadState newState = (ThreadState)(oldState | TS_SyncSuspended); - if (FastInterlockCompareExchange((LONG *)&m_State, newState, oldState) == (LONG)oldState) - { - result = m_UserSuspendEvent.Wait(INFINITE,FALSE); -#if _DEBUG - newState = m_State; - _ASSERTE(!(newState & TS_SyncSuspended) || (newState & TS_DebugSuspendPending)); -#endif - break; - } - - oldState = m_State; - } - - - } else if (m_State & TS_DebugSuspendPending) { + if (m_State & TS_DebugSuspendPending) { ThreadState oldState = m_State; @@ -6127,6 +6099,9 @@ void Thread::WaitSuspendEvents(BOOL fDoWait) ThreadState oldState = m_State; + // CoreCLR does not support user-requested thread suspension + _ASSERTE(!(oldState & TS_UserSuspendPending)); + // // If all reasons to suspend are off, we think we can exit // this loop, but we need to check atomically. @@ -7106,9 +7081,9 @@ void Thread::MarkForSuspension(ULONG bit) } CONTRACTL_END; + // CoreCLR does not support user-requested thread suspension _ASSERTE(bit == TS_DebugSuspendPending || - bit == (TS_DebugSuspendPending | TS_DebugWillSync) || - bit == TS_UserSuspendPending); + bit == (TS_DebugSuspendPending | TS_DebugWillSync)); _ASSERTE(IsAtProcessExit() || ThreadStore::HoldingThreadStore()); @@ -7126,8 +7101,8 @@ void Thread::UnmarkForSuspension(ULONG mask) } CONTRACTL_END; - _ASSERTE(mask == ~TS_DebugSuspendPending || - mask == ~TS_UserSuspendPending); + // CoreCLR does not support user-requested thread suspension + _ASSERTE(mask == ~TS_DebugSuspendPending); _ASSERTE(IsAtProcessExit() || ThreadStore::HoldingThreadStore()); diff --git a/src/coreclr/tests/src/baseservices/threading/DeadThreads/DeadThreads.cs b/src/coreclr/tests/src/baseservices/threading/DeadThreads/DeadThreads.cs new file mode 100644 index 0000000..cc9f054 --- /dev/null +++ b/src/coreclr/tests/src/baseservices/threading/DeadThreads/DeadThreads.cs @@ -0,0 +1,43 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Runtime.CompilerServices; +using System.Threading; + +public class DeadThreads +{ + /// + /// A sanity test that exercises code paths relevant to the heuristic that triggers GCs based on dead thread count and time + /// elapsed since a previous GC. See https://github.com/dotnet/coreclr/pull/10413. + /// + /// This test suite runs with the following environment variables relevant to this test (see .csproj): + /// set COMPlus_Thread_DeadThreadCountThresholdForGCTrigger=8 + /// set COMPlus_Thread_DeadThreadGCTriggerPeriodMilliseconds=3e8 // 1000 + /// + private static void GCTriggerSanityTest() + { + var testDuration = TimeSpan.FromSeconds(8); + var startTime = DateTime.UtcNow; + do + { + StartNoOpThread(); + Thread.Sleep(1); + } while (DateTime.UtcNow - startTime < testDuration); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void StartNoOpThread() + { + var t = new Thread(() => { }); + t.IsBackground = true; + t.Start(); + } + + public static int Main() + { + GCTriggerSanityTest(); + return 100; + } +} diff --git a/src/coreclr/tests/src/baseservices/threading/DeadThreads/DeadThreads.csproj b/src/coreclr/tests/src/baseservices/threading/DeadThreads/DeadThreads.csproj new file mode 100644 index 0000000..b9c2758 --- /dev/null +++ b/src/coreclr/tests/src/baseservices/threading/DeadThreads/DeadThreads.csproj @@ -0,0 +1,32 @@ + + + + + Debug + AnyCPU + DeadThreads + {649A239B-AB4B-4E20-BDCA-3BA736E8B790} + Exe + + + + + + + + + + + + + + + -- 2.7.4