Setting the m_willBlockOnGarbageCollectionEvent earlier and only when we will block...

Setting the m_willBlockOnGarbageCollectionEvent earlier and only when we will block to avoid the async break deadlock

diff --git a/src/debug/ee/debugger.cpp b/src/debug/ee/debugger.cpp
index 5796a36..7031b55 100644 (file)
--- a/src/debug/ee/debugger.cpp
+++ b/src/debug/ee/debugger.cpp
@@ -951,6 +951,7 @@ Debugger::Debugger()
     m_pLazyData(NULL),
     m_defines(_defines),
     m_isBlockedOnGarbageCollectionEvent(FALSE),
+    m_willBlockOnGarbageCollectionEvent(FALSE),
     m_isGarbageCollectionEventsEnabled(FALSE),
     m_isGarbageCollectionEventsEnabledLatch(FALSE)
 {
@@ -5989,7 +5990,7 @@ bool Debugger::ThreadsAtUnsafePlaces(void)
     return (m_threadsAtUnsafePlaces != 0);
 }
 
-void Debugger::BeforeGarbageCollection()
+void Debugger::SuspendForGarbageCollectionStarted()
 {
     CONTRACTL
     {
@@ -5997,13 +5998,25 @@ void Debugger::BeforeGarbageCollection()
         GC_NOTRIGGER;
     }
     CONTRACTL_END;
-
+    
     this->m_isGarbageCollectionEventsEnabledLatch = this->m_isGarbageCollectionEventsEnabled;
+    this->m_willBlockOnGarbageCollectionEvent = this->m_isGarbageCollectionEventsEnabledLatch;
+}
+
+void Debugger::SuspendForGarbageCollectionCompleted()
+{
+    CONTRACTL
+    {
+        NOTHROW;
+        GC_NOTRIGGER;
+    }
+    CONTRACTL_END;
 
     if (!CORDebuggerAttached() || !this->m_isGarbageCollectionEventsEnabledLatch)
     {
         return;
     }
+    this->m_isBlockedOnGarbageCollectionEvent = TRUE; 
 
     Thread* pThread = GetThread();
 
@@ -6013,8 +6026,6 @@ void Debugger::BeforeGarbageCollection()
     {
         Debugger::DebuggerLockHolder dbgLockHolder(this);
 
-        this->m_isBlockedOnGarbageCollectionEvent = true;
-
         DebuggerIPCEvent* ipce1 = m_pRCThread->GetIPCEventSendBuffer();
         InitIPCEvent(ipce1,
             DB_IPCE_BEFORE_GARBAGE_COLLECTION,
@@ -6029,7 +6040,7 @@ void Debugger::BeforeGarbageCollection()
     ResetEvent(this->GetGarbageCollectionBlockerEvent());
 }
 
-void Debugger::AfterGarbageCollection()
+void Debugger::ResumeForGarbageCollectionStarted()
 {
     CONTRACTL
     {
@@ -6063,7 +6074,8 @@ void Debugger::AfterGarbageCollection()
 
     WaitForSingleObject(this->GetGarbageCollectionBlockerEvent(), INFINITE);
     ResetEvent(this->GetGarbageCollectionBlockerEvent());
-    this->m_isBlockedOnGarbageCollectionEvent = false;
+    this->m_isBlockedOnGarbageCollectionEvent = FALSE;
+    this->m_willBlockOnGarbageCollectionEvent = FALSE;
 }
 
 #ifdef FEATURE_DATABREAKPOINT
@@ -10738,9 +10750,9 @@ bool Debugger::HandleIPCEvent(DebuggerIPCEvent * pEvent)
 
     if ((pEvent->type & DB_IPCE_TYPE_MASK) == DB_IPCE_ASYNC_BREAK ||
         (pEvent->type & DB_IPCE_TYPE_MASK) == DB_IPCE_ATTACHING ||
-        this->m_isBlockedOnGarbageCollectionEvent)
+        this->m_willBlockOnGarbageCollectionEvent)
     {
-        if (!this->m_isBlockedOnGarbageCollectionEvent)
+        if (!this->m_willBlockOnGarbageCollectionEvent && !this->m_stopped)
         {
             lockedThreadStore = true;
             ThreadSuspend::LockThreadStore(ThreadSuspend::SUSPEND_FOR_DEBUGGER);
@@ -10807,7 +10819,7 @@ bool Debugger::HandleIPCEvent(DebuggerIPCEvent * pEvent)
                 _ASSERTE(ThreadHoldsLock());
 
                 // Simply trap all Runtime threads if we're not already trying to.
-                if (!m_isBlockedOnGarbageCollectionEvent && !m_trappingRuntimeThreads)
+                if (!m_willBlockOnGarbageCollectionEvent && !m_trappingRuntimeThreads)
                 {
                     // If the RS sent an Async-break, then that's an explicit request.
                     m_RSRequestedSync = TRUE;

diff --git a/src/debug/ee/debugger.h b/src/debug/ee/debugger.h
index 746f914..f2271d2 100644 (file)
--- a/src/debug/ee/debugger.h
+++ b/src/debug/ee/debugger.h
@@ -2954,10 +2954,12 @@ public:
     DWORD m_defines;
     DWORD m_mdDataStructureVersion;
 #ifndef DACCESS_COMPILE
-    virtual void BeforeGarbageCollection();
-    virtual void AfterGarbageCollection();
+    virtual void SuspendForGarbageCollectionStarted();
+    virtual void SuspendForGarbageCollectionCompleted();
+    virtual void ResumeForGarbageCollectionStarted();
 #endif
     BOOL m_isBlockedOnGarbageCollectionEvent;
+    BOOL m_willBlockOnGarbageCollectionEvent;
     BOOL m_isGarbageCollectionEventsEnabled;
     // this latches m_isGarbageCollectionEventsEnabled in BeforeGarbageCollection so we can 
     // guarantee the corresponding AfterGC event is sent even if the events are disabled during GC.

diff --git a/src/vm/dbginterface.h b/src/vm/dbginterface.h
index d0be137..9c5ae0d 100644 (file)
--- a/src/vm/dbginterface.h
+++ b/src/vm/dbginterface.h
@@ -409,8 +409,9 @@ public:
     virtual void EnumMemoryRegionsIfFuncEvalFrame(CLRDataEnumMemoryFlags flags, Frame * pFrame) = 0;
 #endif
 #ifndef DACCESS_COMPILE
-    virtual void BeforeGarbageCollection() = 0;
-    virtual void AfterGarbageCollection() = 0;
+    virtual void SuspendForGarbageCollectionStarted() = 0;
+    virtual void SuspendForGarbageCollectionCompleted() = 0;
+    virtual void ResumeForGarbageCollectionStarted() = 0;
 #endif
     virtual BOOL IsSynchronizing() = 0;
 };

diff --git a/src/vm/gcenv.ee.cpp b/src/vm/gcenv.ee.cpp
index ff8f109..9898542 100644 (file)
--- a/src/vm/gcenv.ee.cpp
+++ b/src/vm/gcenv.ee.cpp
@@ -20,16 +20,18 @@ void GCToEEInterface::SuspendEE(SUSPEND_REASON reason)
 
     _ASSERTE(reason == SUSPEND_FOR_GC || reason == SUSPEND_FOR_GC_PREP);
 
+    g_pDebugInterface->SuspendForGarbageCollectionStarted();
+
     ThreadSuspend::SuspendEE((ThreadSuspend::SUSPEND_REASON)reason);
 
-    g_pDebugInterface->BeforeGarbageCollection();
+    g_pDebugInterface->SuspendForGarbageCollectionCompleted();
 }
 
 void GCToEEInterface::RestartEE(bool bFinishedGC)
 {
     WRAPPER_NO_CONTRACT;
 
-    g_pDebugInterface->AfterGarbageCollection();
+    g_pDebugInterface->ResumeForGarbageCollectionStarted();
 
     ThreadSuspend::RestartEE(bFinishedGC, TRUE);
 }