From 2c2ad5154d9dfb42124557a94e7e81c0b810755b Mon Sep 17 00:00:00 2001 From: Andrew Au Date: Tue, 22 May 2018 15:08:25 -0700 Subject: [PATCH] Firing the GC events within the thread suspension Commit migrated from https://github.com/dotnet/coreclr/commit/c5a69820e866fff47c9f415d872112c58f17a717 --- src/coreclr/src/debug/ee/debugger.h | 4 + src/coreclr/src/debug/ee/rcthread.cpp | 10 +- src/coreclr/src/inc/CrstTypes.def | 4 +- src/coreclr/src/inc/crsttypes.h | 346 +++++++++++++++++----------------- src/coreclr/src/vm/gcenv.ee.cpp | 10 +- src/coreclr/src/vm/threads.cpp | 14 +- src/coreclr/src/vm/threads.h | 2 + 7 files changed, 206 insertions(+), 184 deletions(-) diff --git a/src/coreclr/src/debug/ee/debugger.h b/src/coreclr/src/debug/ee/debugger.h index 97f9832..1db799c 100644 --- a/src/coreclr/src/debug/ee/debugger.h +++ b/src/coreclr/src/debug/ee/debugger.h @@ -3791,12 +3791,14 @@ HANDLE OpenWin32EventOrThrow( #define SENDIPCEVENT_RAW_BEGIN_EX(pDbgLockHolder, gcxStmt) \ { \ + ThreadStore::EnterThreadStoreLockOnly(); \ Debugger::DebuggerLockHolder *__pDbgLockHolder = pDbgLockHolder; \ gcxStmt; \ g_pDebugger->LockForEventSending(__pDbgLockHolder); #define SENDIPCEVENT_RAW_END_EX \ g_pDebugger->UnlockFromEventSending(__pDbgLockHolder); \ + ThreadStore::LeaveThreadStoreLockOnly(); \ } #define SENDIPCEVENT_RAW_BEGIN(pDbgLockHolder) \ @@ -3821,6 +3823,7 @@ HANDLE OpenWin32EventOrThrow( Debugger::DebuggerLockHolder __dbgLockHolder(pDebugger, FALSE); \ Debugger::DebuggerLockHolder *__pDbgLockHolder = &__dbgLockHolder; \ gcxStmt; \ + ThreadStore::EnterThreadStoreLockOnly(); \ g_pDebugger->LockForEventSending(__pDbgLockHolder); \ /* Check if the thread has been suspended by the debugger via SetDebugState(). */ \ if (thread != NULL && thread->HasThreadStateNC(Thread::TSNC_DebuggerUserSuspend)) \ @@ -3835,6 +3838,7 @@ HANDLE OpenWin32EventOrThrow( ; \ } \ g_pDebugger->UnlockFromEventSending(__pDbgLockHolder); \ + ThreadStore::LeaveThreadStoreLockOnly(); \ } /* ~gcxStmt & ~DebuggerLockHolder */ \ } while (__fRetry); \ FireEtwDebugIPCEventEnd(); \ diff --git a/src/coreclr/src/debug/ee/rcthread.cpp b/src/coreclr/src/debug/ee/rcthread.cpp index 2d1b06d..06cc17e 100644 --- a/src/coreclr/src/debug/ee/rcthread.cpp +++ b/src/coreclr/src/debug/ee/rcthread.cpp @@ -11,7 +11,7 @@ //***************************************************************************** #include "stdafx.h" - +#include "threadsuspend.h" #ifndef FEATURE_PAL #include "securitywrapper.h" #endif @@ -1226,6 +1226,7 @@ void DebuggerRCThread::MainLoop() // Let's release the lock here since runtime is resumed. debugLockHolderSuspended.Release(); + ThreadStore::LeaveThreadStoreLockOnly(); // This debugger thread shoud not be holding debugger locks anymore _ASSERTE(!g_pDebugger->ThreadHoldsLock()); @@ -1247,7 +1248,8 @@ void DebuggerRCThread::MainLoop() else if (dwWaitResult == WAIT_OBJECT_0 + DRCT_CONTROL_EVENT) { LOG((LF_CORDB, LL_INFO1000, "DRCT::ML:: straggler event set.\n")); - + + ThreadStore::EnterThreadStoreLockOnly(); Debugger::DebuggerLockHolder debugLockHolder(m_debugger); // Make sure that we're still synchronizing... if (m_debugger->IsSynchronizing()) @@ -1260,12 +1262,14 @@ void DebuggerRCThread::MainLoop() // Skip waiting the first time and just give it a go. Note: Implicit // release of the lock, because we are leaving its scope. // + ThreadStore::LeaveThreadStoreLockOnly(); goto LWaitTimedOut; } #ifdef LOGGING else LOG((LF_CORDB, LL_INFO1000, "DRCT::ML:: told to wait, but not syncing anymore.\n")); #endif + ThreadStore::LeaveThreadStoreLockOnly(); // dbgLockHolder goes out of scope - implicit Release } else if (dwWaitResult == WAIT_TIMEOUT) @@ -1275,6 +1279,7 @@ LWaitTimedOut: LOG((LF_CORDB, LL_INFO1000, "DRCT::ML:: wait timed out.\n")); + ThreadStore::EnterThreadStoreLockOnly(); // Debugger::DebuggerLockHolder debugLockHolder(m_debugger); // Explicitly get the lock here since we try to check to see if // have suspended. We will release the lock if we are not suspended yet. @@ -1329,6 +1334,7 @@ LWaitTimedOut: // And so the sweep should always succeed. STRESS_LOG0(LF_CORDB, LL_INFO1000, "DRCT::ML:: threads still syncing after sweep.\n"); debugLockHolderSuspended.Release(); + ThreadStore::LeaveThreadStoreLockOnly(); } // debugLockHolderSuspended does not go out of scope. It has to be either released explicitly on the line above or // we intend to hold the lock till we hit continue event. diff --git a/src/coreclr/src/inc/CrstTypes.def b/src/coreclr/src/inc/CrstTypes.def index a0ee344..c08ee70 100644 --- a/src/coreclr/src/inc/CrstTypes.def +++ b/src/coreclr/src/inc/CrstTypes.def @@ -207,7 +207,7 @@ End // It's the largest of the debugger locks. Crst DebuggerMutex AcquiredBefore AvailableParamTypes ConnectionNameTable - DynamicIL LoaderHeap ModuleLookupTable ThreadStore + DynamicIL LoaderHeap ModuleLookupTable // Disabled per bug 581892 // AcquiredBefore DebuggerController @@ -719,7 +719,7 @@ Crst ThreadStore DebuggerHeapLock DebuggerJitInfo DynamicIL ExecuteManRangeLock HandleTable IbcProfile JitGenericHandleCache JumpStubCache LoaderHeap ModuleLookupTable ProfilingAPIStatus ProfilerGCRefDataFreeList RWLock SingleUseLock SyncBlockCache SystemDomainDelayedUnloadList - ThreadIdDispenser ThreadStaticDataHashTable + ThreadIdDispenser ThreadStaticDataHashTable DebuggerMutex End Crst TPMethodTable diff --git a/src/coreclr/src/inc/crsttypes.h b/src/coreclr/src/inc/crsttypes.h index 0985b9f..7a622f1 100644 --- a/src/coreclr/src/inc/crsttypes.h +++ b/src/coreclr/src/inc/crsttypes.h @@ -200,179 +200,179 @@ enum CrstType // An array mapping CrstType to level. int g_rgCrstLevelMap[] = { - 9, // CrstAllowedFiles - 9, // CrstAppDomainCache - 13, // CrstAppDomainHandleTable - 0, // CrstArgBasedStubCache - 0, // CrstAssemblyDependencyGraph - 0, // CrstAssemblyIdentityCache - 0, // CrstAssemblyList - 7, // CrstAssemblyLoader - 3, // CrstAvailableClass - 6, // CrstAvailableParamTypes - 7, // CrstBaseDomain - -1, // CrstCCompRC - 9, // CrstCer - 11, // CrstClassFactInfoHash - 8, // CrstClassInit - -1, // CrstClrNotification - 0, // CrstCLRPrivBinderMaps - 3, // CrstCLRPrivBinderMapsAdd - 6, // CrstCodeFragmentHeap - 4, // CrstCOMWrapperCache - 0, // CrstConnectionNameTable - 17, // CrstContexts - -1, // CrstCoreCLRBinderLog - 0, // CrstCrstCLRPrivBinderLocalWinMDPath - 7, // CrstCSPCache - 3, // CrstDataTest1 - 0, // CrstDataTest2 - 0, // CrstDbgTransport - 0, // CrstDeadlockDetection - -1, // CrstDebuggerController - 3, // CrstDebuggerFavorLock - 0, // CrstDebuggerHeapExecMemLock - 0, // CrstDebuggerHeapLock - 4, // CrstDebuggerJitInfo - 11, // CrstDebuggerMutex - 0, // CrstDelegateToFPtrHash - 15, // CrstDomainLocalBlock - 0, // CrstDynamicIL - 3, // CrstDynamicMT - 3, // CrstDynLinkZapItems - 7, // CrstEtwTypeLogHash - 17, // CrstEventPipe - 0, // CrstEventStore - 0, // CrstException - 7, // CrstExecuteManLock - 0, // CrstExecuteManRangeLock - 3, // CrstFCall - 7, // CrstFriendAccessCache - 7, // CrstFuncPtrStubs - 9, // CrstFusionAppCtx - 7, // CrstFusionAssemblyDownload - 5, // CrstFusionBindContext - 0, // CrstFusionBindResult - 0, // CrstFusionClb - 16, // CrstFusionClosure - 10, // CrstFusionClosureGraph - 0, // CrstFusionConfigSettings - 0, // CrstFusionDownload - 0, // CrstFusionIsoLibInit - 5, // CrstFusionLoadContext - 4, // CrstFusionLog - 7, // CrstFusionNgenIndex - 7, // CrstFusionNgenIndexPool - 0, // CrstFusionPcyCache - 4, // CrstFusionPolicyConfigPool - 5, // CrstFusionSingleUse - 6, // CrstFusionWarningLog - 10, // CrstGCCover - 0, // CrstGCMemoryPressure - 11, // CrstGlobalStrLiteralMap - 1, // CrstHandleTable - 0, // CrstHostAssemblyMap - 3, // CrstHostAssemblyMapAdd - 0, // CrstIbcProfile - 9, // CrstIJWFixupData - 0, // CrstIJWHash - 5, // CrstILFingerprintCache - 7, // CrstILStubGen - 3, // CrstInlineTrackingMap - 16, // CrstInstMethodHashTable - 0, // CrstInterfaceVTableMap - 17, // CrstInterop - 4, // CrstInteropData - 11, // CrstIOThreadpoolWorker - 0, // CrstIsJMCMethod - 7, // CrstISymUnmanagedReader - 8, // CrstJit - 0, // CrstJitGenericHandleCache - -1, // CrstJitPerf - 6, // CrstJumpStubCache - 0, // CrstLeafLock - -1, // CrstListLock - 14, // CrstLoaderAllocator - 15, // CrstLoaderAllocatorReferences - 0, // CrstLoaderHeap - 0, // CrstMda - -1, // CrstMetadataTracker - 0, // CrstModIntPairList - 4, // CrstModule - 14, // CrstModuleFixup - 3, // CrstModuleLookupTable - 0, // CrstMulticoreJitHash - 11, // CrstMulticoreJitManager - 0, // CrstMUThunkHash - -1, // CrstNativeBinderInit - -1, // CrstNativeImageCache - 0, // CrstNls - 0, // CrstNotifyGdb - 2, // CrstObjectList - 0, // CrstOnEventManager - 0, // CrstPatchEntryPoint - 0, // CrstPEFileSecurityManager - 4, // CrstPEImage - 0, // CrstPEImagePDBStream - 18, // CrstPendingTypeLoadEntry - 0, // CrstPinHandle - 0, // CrstPinnedByrefValidation - 0, // CrstProfilerGCRefDataFreeList - 0, // CrstProfilingAPIStatus - 0, // CrstPublisherCertificate - 3, // CrstRCWCache - 0, // CrstRCWCleanupList - 3, // CrstRCWRefCache - 4, // CrstReadyToRunEntryPointToMethodDescMap - 0, // CrstReDacl - 9, // CrstReflection - 7, // CrstReJITDomainTable - 13, // CrstReJITGlobalRequest - 9, // CrstReJITSharedDomainTable - 19, // CrstRemoting - 3, // CrstRetThunkCache - 0, // CrstRWLock - 3, // CrstSavedExceptionInfo - 0, // CrstSaveModuleProfileData - 0, // CrstSecurityPolicyCache - 3, // CrstSecurityPolicyInit - 0, // CrstSecurityStackwalkCache - 4, // CrstSharedAssemblyCreate - 7, // CrstSharedBaseDomain - 3, // CrstSigConvert - 5, // CrstSingleUseLock - 0, // CrstSpecialStatics - 0, // CrstSqmManager - 0, // CrstStackSampler - -1, // CrstStressLog - 0, // CrstStrongName - 5, // CrstStubCache - 0, // CrstStubDispatchCache - 4, // CrstStubUnwindInfoHeapSegments - 3, // CrstSyncBlockCache - 0, // CrstSyncHashLock - 0, // CrstSystemBaseDomain - 12, // CrstSystemDomain - 0, // CrstSystemDomainDelayedUnloadList - 0, // CrstThreadIdDispenser - 0, // CrstThreadpoolEventCache - 7, // CrstThreadpoolTimerQueue - 7, // CrstThreadpoolWaitThreads - 11, // CrstThreadpoolWorker - 4, // CrstThreadStaticDataHashTable - 10, // CrstThreadStore - 9, // CrstTieredCompilation - 9, // CrstTPMethodTable - 3, // CrstTypeEquivalenceMap - 7, // CrstTypeIDMap - 3, // CrstUMEntryThunkCache - 0, // CrstUMThunkHash - 3, // CrstUniqueStack - 7, // CrstUnresolvedClassLock - 3, // CrstUnwindInfoTableLock - 3, // CrstVSDIndirectionCellLock - 3, // CrstWinRTFactoryCache - 3, // CrstWrapperTemplate + 9, // CrstAllowedFiles + 9, // CrstAppDomainCache + 13, // CrstAppDomainHandleTable + 0, // CrstArgBasedStubCache + 0, // CrstAssemblyDependencyGraph + 0, // CrstAssemblyIdentityCache + 0, // CrstAssemblyList + 7, // CrstAssemblyLoader + 3, // CrstAvailableClass + 6, // CrstAvailableParamTypes + 7, // CrstBaseDomain + -1, // CrstCCompRC + 9, // CrstCer + 12, // CrstClassFactInfoHash + 8, // CrstClassInit + -1, // CrstClrNotification + 0, // CrstCLRPrivBinderMaps + 3, // CrstCLRPrivBinderMapsAdd + 6, // CrstCodeFragmentHeap + 4, // CrstCOMWrapperCache + 0, // CrstConnectionNameTable + 17, // CrstContexts + -1, // CrstCoreCLRBinderLog + 0, // CrstCrstCLRPrivBinderLocalWinMDPath + 7, // CrstCSPCache + 3, // CrstDataTest1 + 0, // CrstDataTest2 + 0, // CrstDbgTransport + 0, // CrstDeadlockDetection + -1, // CrstDebuggerController + 3, // CrstDebuggerFavorLock + 0, // CrstDebuggerHeapExecMemLock + 0, // CrstDebuggerHeapLock + 4, // CrstDebuggerJitInfo + 10, // CrstDebuggerMutex + 0, // CrstDelegateToFPtrHash + 15, // CrstDomainLocalBlock + 0, // CrstDynamicIL + 3, // CrstDynamicMT + 3, // CrstDynLinkZapItems + 7, // CrstEtwTypeLogHash + 17, // CrstEventPipe + 0, // CrstEventStore + 0, // CrstException + 7, // CrstExecuteManLock + 0, // CrstExecuteManRangeLock + 3, // CrstFCall + 7, // CrstFriendAccessCache + 7, // CrstFuncPtrStubs + 9, // CrstFusionAppCtx + 7, // CrstFusionAssemblyDownload + 5, // CrstFusionBindContext + 0, // CrstFusionBindResult + 0, // CrstFusionClb + 16, // CrstFusionClosure + 10, // CrstFusionClosureGraph + 0, // CrstFusionConfigSettings + 0, // CrstFusionDownload + 0, // CrstFusionIsoLibInit + 5, // CrstFusionLoadContext + 4, // CrstFusionLog + 7, // CrstFusionNgenIndex + 7, // CrstFusionNgenIndexPool + 0, // CrstFusionPcyCache + 4, // CrstFusionPolicyConfigPool + 5, // CrstFusionSingleUse + 6, // CrstFusionWarningLog + 10, // CrstGCCover + 0, // CrstGCMemoryPressure + 12, // CrstGlobalStrLiteralMap + 1, // CrstHandleTable + 0, // CrstHostAssemblyMap + 3, // CrstHostAssemblyMapAdd + 0, // CrstIbcProfile + 9, // CrstIJWFixupData + 0, // CrstIJWHash + 5, // CrstILFingerprintCache + 7, // CrstILStubGen + 3, // CrstInlineTrackingMap + 16, // CrstInstMethodHashTable + 0, // CrstInterfaceVTableMap + 17, // CrstInterop + 4, // CrstInteropData + 12, // CrstIOThreadpoolWorker + 0, // CrstIsJMCMethod + 7, // CrstISymUnmanagedReader + 8, // CrstJit + 0, // CrstJitGenericHandleCache + -1, // CrstJitPerf + 6, // CrstJumpStubCache + 0, // CrstLeafLock + -1, // CrstListLock + 14, // CrstLoaderAllocator + 15, // CrstLoaderAllocatorReferences + 0, // CrstLoaderHeap + 0, // CrstMda + -1, // CrstMetadataTracker + 0, // CrstModIntPairList + 4, // CrstModule + 14, // CrstModuleFixup + 3, // CrstModuleLookupTable + 0, // CrstMulticoreJitHash + 12, // CrstMulticoreJitManager + 0, // CrstMUThunkHash + -1, // CrstNativeBinderInit + -1, // CrstNativeImageCache + 0, // CrstNls + 0, // CrstNotifyGdb + 2, // CrstObjectList + 0, // CrstOnEventManager + 0, // CrstPatchEntryPoint + 0, // CrstPEFileSecurityManager + 4, // CrstPEImage + 0, // CrstPEImagePDBStream + 18, // CrstPendingTypeLoadEntry + 0, // CrstPinHandle + 0, // CrstPinnedByrefValidation + 0, // CrstProfilerGCRefDataFreeList + 0, // CrstProfilingAPIStatus + 0, // CrstPublisherCertificate + 3, // CrstRCWCache + 0, // CrstRCWCleanupList + 3, // CrstRCWRefCache + 4, // CrstReadyToRunEntryPointToMethodDescMap + 0, // CrstReDacl + 9, // CrstReflection + 7, // CrstReJITDomainTable + 13, // CrstReJITGlobalRequest + 9, // CrstReJITSharedDomainTable + 19, // CrstRemoting + 3, // CrstRetThunkCache + 0, // CrstRWLock + 3, // CrstSavedExceptionInfo + 0, // CrstSaveModuleProfileData + 0, // CrstSecurityPolicyCache + 3, // CrstSecurityPolicyInit + 0, // CrstSecurityStackwalkCache + 4, // CrstSharedAssemblyCreate + 7, // CrstSharedBaseDomain + 3, // CrstSigConvert + 5, // CrstSingleUseLock + 0, // CrstSpecialStatics + 0, // CrstSqmManager + 0, // CrstStackSampler + -1, // CrstStressLog + 0, // CrstStrongName + 5, // CrstStubCache + 0, // CrstStubDispatchCache + 4, // CrstStubUnwindInfoHeapSegments + 3, // CrstSyncBlockCache + 0, // CrstSyncHashLock + 0, // CrstSystemBaseDomain + 12, // CrstSystemDomain + 0, // CrstSystemDomainDelayedUnloadList + 0, // CrstThreadIdDispenser + 0, // CrstThreadpoolEventCache + 7, // CrstThreadpoolTimerQueue + 7, // CrstThreadpoolWaitThreads + 12, // CrstThreadpoolWorker + 4, // CrstThreadStaticDataHashTable + 11, // CrstThreadStore + 9, // CrstTieredCompilation + 9, // CrstTPMethodTable + 3, // CrstTypeEquivalenceMap + 7, // CrstTypeIDMap + 3, // CrstUMEntryThunkCache + 0, // CrstUMThunkHash + 3, // CrstUniqueStack + 7, // CrstUnresolvedClassLock + 3, // CrstUnwindInfoTableLock + 3, // CrstVSDIndirectionCellLock + 3, // CrstWinRTFactoryCache + 3, // CrstWrapperTemplate }; // An array mapping CrstType to a stringized name. diff --git a/src/coreclr/src/vm/gcenv.ee.cpp b/src/coreclr/src/vm/gcenv.ee.cpp index 99b0817..87b3f93 100644 --- a/src/coreclr/src/vm/gcenv.ee.cpp +++ b/src/coreclr/src/vm/gcenv.ee.cpp @@ -20,20 +20,18 @@ void GCToEEInterface::SuspendEE(SUSPEND_REASON reason) _ASSERTE(reason == SUSPEND_FOR_GC || reason == SUSPEND_FOR_GC_PREP); - // TODO, between the time when the debug event is sent and the EE is suspended, there is a small window that the data breakpoint could have already hit - g_pDebugInterface->BeforeGarbageCollection(); - ThreadSuspend::SuspendEE((ThreadSuspend::SUSPEND_REASON)reason); + + g_pDebugInterface->BeforeGarbageCollection(); } void GCToEEInterface::RestartEE(bool bFinishedGC) { WRAPPER_NO_CONTRACT; - ThreadSuspend::RestartEE(bFinishedGC, TRUE); - - // TODO, between the time when the debug event is sent and the EE is suspended, there is a small window that the data breakpoint could have already hit g_pDebugInterface->AfterGarbageCollection(); + + ThreadSuspend::RestartEE(bFinishedGC, TRUE); } VOID GCToEEInterface::SyncBlockCacheWeakPtrScan(HANDLESCANPROC scanProc, uintptr_t lp1, uintptr_t lp2) diff --git a/src/coreclr/src/vm/threads.cpp b/src/coreclr/src/vm/threads.cpp index 7a638a9..15e36ca 100644 --- a/src/coreclr/src/vm/threads.cpp +++ b/src/coreclr/src/vm/threads.cpp @@ -5349,7 +5349,7 @@ Thread::ApartmentState Thread::SetApartment(ApartmentState state, BOOL fFireMDAO //---------------------------------------------------------------------------- ThreadStore::ThreadStore() - : m_Crst(CrstThreadStore, (CrstFlags) (CRST_UNSAFE_ANYMODE | CRST_DEBUGGER_THREAD)), + : m_Crst(CrstThreadStore, (CrstFlags) (CRST_UNSAFE_ANYMODE | CRST_REENTRANCY | CRST_DEBUGGER_THREAD)), m_ThreadCount(0), m_MaxThreadCount(0), m_UnstartedThreadCount(0), @@ -5434,6 +5434,18 @@ DEBUG_NOINLINE void ThreadStore::Leave() m_Crst.Leave(); } +void ThreadStore::EnterThreadStoreLockOnly() +{ + WRAPPER_NO_CONTRACT; + s_pThreadStore->Enter(); +} + +void ThreadStore::LeaveThreadStoreLockOnly() +{ + WRAPPER_NO_CONTRACT; + s_pThreadStore->Leave(); +} + void ThreadStore::LockThreadStore() { WRAPPER_NO_CONTRACT; diff --git a/src/coreclr/src/vm/threads.h b/src/coreclr/src/vm/threads.h index 3aa1681..3d9db05 100644 --- a/src/coreclr/src/vm/threads.h +++ b/src/coreclr/src/vm/threads.h @@ -5321,6 +5321,8 @@ public: static void InitThreadStore(); static void LockThreadStore(); static void UnlockThreadStore(); + static void EnterThreadStoreLockOnly(); + static void LeaveThreadStoreLockOnly(); // Add a Thread to the ThreadStore // WARNING : only GC calls this with bRequiresTSL set to FALSE. -- 2.7.4