Add heuristic to trigger GC to finalize dead threads and clean up han… (dotnet/corecl...
authorKoundinya Veluri <kouvel@microsoft.com>
Mon, 27 Mar 2017 18:10:07 +0000 (11:10 -0700)
committerGitHub <noreply@github.com>
Mon, 27 Mar 2017 18:10:07 +0000 (11:10 -0700)
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

12 files changed:
src/coreclr/src/debug/daccess/dacdbiimpl.cpp
src/coreclr/src/inc/clrconfigvalues.h
src/coreclr/src/vm/comsynchronizable.cpp
src/coreclr/src/vm/debugdebugger.cpp
src/coreclr/src/vm/eedbginterfaceimpl.cpp
src/coreclr/src/vm/finalizerthread.cpp
src/coreclr/src/vm/gcenv.ee.cpp
src/coreclr/src/vm/threads.cpp
src/coreclr/src/vm/threads.h
src/coreclr/src/vm/threadsuspend.cpp
src/coreclr/tests/src/baseservices/threading/DeadThreads/DeadThreads.cs [new file with mode: 0644]
src/coreclr/tests/src/baseservices/threading/DeadThreads/DeadThreads.csproj [new file with mode: 0644]

index c7595c4..74ba14f 100644 (file)
@@ -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())
     {
index 6160f20..b24ef39 100644 (file)
@@ -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
 // 
index b8f871d..08b5281 100644 (file)
@@ -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();
index 32c6cd6..489c623 100644 (file)
@@ -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);
 
index b7a4359..ede82c7 100644 (file)
@@ -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));
index 51a2ed4..d111eeb 100644 (file)
@@ -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)
index bf2ce2c..392c179 100644 (file)
@@ -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)
index b5f7507..6d9d10b 100644 (file)
@@ -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<LONG>(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<SIZE_T>(FastInterlockIncrement(&m_DeadThreadCountForGCTrigger));
+
+    SIZE_T countThreshold = static_cast<SIZE_T>(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<SIZE_T>(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<int>(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();
 }
 
 
index ff0d166..f34066f 100644 (file)
@@ -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 {
index 10032fd..71409ce 100644 (file)
@@ -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();
 
-            // <REVISIT_TODO>@TODO: Is this necessary?  Does debugger wait on the events, or does it just
-            //        poll every so often?</REVISIT_TODO>
-            // 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 (file)
index 0000000..cc9f054
--- /dev/null
@@ -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
+{
+    /// <summary>
+    /// 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
+    /// </summary>
+    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 (file)
index 0000000..b9c2758
--- /dev/null
@@ -0,0 +1,32 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
+  <PropertyGroup>
+    <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
+    <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
+    <AssemblyName>DeadThreads</AssemblyName>
+    <ProjectGuid>{649A239B-AB4B-4E20-BDCA-3BA736E8B790}</ProjectGuid>
+    <OutputType>Exe</OutputType>
+  </PropertyGroup>
+  <!-- Default configurations to help VS understand the configurations -->
+  <PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'Debug|x64'">
+  </PropertyGroup>
+  <PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'Release|x64'">
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="DeadThreads.cs" />
+  </ItemGroup>
+  <PropertyGroup>
+    <CLRTestBatchPreCommands><![CDATA[
+$(CLRTestBatchPreCommands)
+set COMPlus_Thread_DeadThreadCountThresholdForGCTrigger=8
+set COMPlus_Thread_DeadThreadGCTriggerPeriodMilliseconds=3e8
+]]></CLRTestBatchPreCommands>
+    <BashCLRTestPreCommands><![CDATA[
+$(BashCLRTestPreCommands)
+export COMPlus_Thread_DeadThreadCountThresholdForGCTrigger=8
+export COMPlus_Thread_DeadThreadGCTriggerPeriodMilliseconds=3e8
+]]></BashCLRTestPreCommands>
+  </PropertyGroup>
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+</Project>