Continuously track active dispatched exceptions and flares sent

Author: tommcdon

src/coreclr/debug/di/process.cpp

@@ -1341,6 +1341,8 @@ void CordbProcess::Neuter()
     }
     m_unmanagedThreadHashTable.RemoveAll();
     m_dwOutOfProcessStepping = 0;
+    m_cProcessedFlares = 0;
+    m_fDetachInProgress = FALSE;
 #endif
 
     NeuterChildren();
@@ -3131,7 +3133,6 @@ void CordbProcess::DetachShim()
             DetachInProgressGuard(CordbProcess * pProcess) 
             {
                 m_pProcess = pProcess;
-                pProcess->m_cProcessedFlares = 0;
                 pProcess->m_fDetachInProgress = TRUE;
             }
             ~DetachInProgressGuard() 

src/coreclr/debug/di/rspriv.h

@@ -3324,11 +3324,11 @@ class CordbProcess :
         DetachNotifications(CordbProcess * process) : m_process(process) { }
         ~DetachNotifications()
         {
+            DWORD dwProcessedFlares = InterlockedIncrement(&m_process->m_cProcessedFlares);
             if (m_process->m_fDetachInProgress)
             {
                 // notify that there has been an update to the number of processed 
                 // SetThreadContextNeeded events
-                DWORD dwProcessedFlares = InterlockedIncrement(&m_process->m_cProcessedFlares);
                 LOG((LF_CORDB, LL_INFO10000, "HSTCN: Detach in progress - %d\n", dwProcessedFlares));
                 SetEvent(m_process->m_detachSetThreadContextNeededEvent);
             }

src/coreclr/debug/ee/controller.cpp

@@ -1121,16 +1121,6 @@ DebuggerController::DebuggerController(Thread * pThread, AppDomain * pAppDomain)
     {
         m_next = g_controllers;
         g_controllers = this;
-
-#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT
-        if (DebuggerController::GetProcessingDetach())
-        {
-            // If we are detaching, we need to increment the active dispatched exceptions
-            // so that we can send the detach complete message when we are done.
-            int cActiveDispatchedExceptions = DebuggerController::IncrementActiveDispatchedExceptions();
-            LOG((LF_CORDB, LL_INFO10000, "DC::DC %p incrementing ActiveDispatchedExceptions=%d\n", this, cActiveDispatchedExceptions));
-        }
-#endif // OUT_OF_PROCESS_SETTHREADCONTEXT
     }
 }
 
@@ -1207,20 +1197,14 @@ DebuggerController::~DebuggerController()
 #ifdef OUT_OF_PROCESS_SETTHREADCONTEXT
         if (DebuggerController::GetProcessingDetach())
         {
-            int cActiveDispatchedExceptions = DebuggerController::DecrementActiveDispatchedExceptions();
-            LOG((LF_CORDB, LL_INFO10000, "DC::~DC %p decrementing ActiveDispatchedExceptions=%d\n", this, cActiveDispatchedExceptions));
-
-            if (cActiveDispatchedExceptions == 0)
-            {
-                DebuggerController::SetProcessingDetach(FALSE);
-                sendDetachComplete = true;
-            }
+            sendDetachComplete = CanSendDetach();
         }
 #endif // OUT_OF_PROCESS_SETTHREADCONTEXT
     }
 #ifdef OUT_OF_PROCESS_SETTHREADCONTEXT
     if (sendDetachComplete && g_pDebugger != NULL)
     {
+        LOG((LF_CORDB, LL_INFO1000, "DC::~DC: Sending DetachComplete\n"));
         g_pDebugger->SendDetachComplete();
     }
 #endif
@@ -3095,15 +3079,6 @@ DPOSS_ACTION DebuggerController::DispatchPatchOrSingleStep(Thread *thread, CONTE
     }
     _ASSERTE(g_patches != NULL);
 
-#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT
-    {
-        int cDispatchedExceptions = DebuggerController::IncrementActiveDispatchedExceptions();
-        LOG((LF_CORDB|LF_ENC, LL_INFO10000, "DC::DPOSS Incremented Exception Count = %d\n", cDispatchedExceptions));
-    }
-
-    bool isProcessingDetach = DebuggerController::GetProcessingDetach();
-#endif
-
     CrstHolderWithState lockController(&g_criticalSection);
 
     TADDR originalAddress = 0;
@@ -3169,15 +3144,6 @@ DPOSS_ACTION DebuggerController::DispatchPatchOrSingleStep(Thread *thread, CONTE
     address = (CORDB_ADDRESS_TYPE *)(UINT_PTR)0xAABBCCFF;
 #endif //_DEBUG
 
-#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT
-    if (DebuggerController::GetProcessingDetach() && IsInUsedAction(used))
-    {
-        int cDispatchedFlares = DebuggerController::IncrementDispatchedFlares();
-        isProcessingDetach = true;
-        LOG((LF_CORDB|LF_ENC, LL_INFO10000, "DC::DPOSS Incremented Flare Count = %d\n", cDispatchedFlares));
-    }
-#endif // OUT_OF_PROCESS_SETTHREADCONTEXT
-
     if (dcq.dcqGetCount()> 0)
     {
         lockController.Release();
@@ -3278,14 +3244,6 @@ DPOSS_ACTION DebuggerController::DispatchPatchOrSingleStep(Thread *thread, CONTE
 
     lockController.Release();
 
-#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT
-    if (DebuggerController::GetProcessingDetach() && IsInUsedAction(used) && !isProcessingDetach)
-    {
-        int cDispatchedFlares = DebuggerController::IncrementDispatchedFlares();
-        isProcessingDetach = true;
-        LOG((LF_CORDB|LF_ENC, LL_INFO10000, "DC::DPOSS Incremented Flare Count = %d\n", cDispatchedFlares));
-    }
-#endif // OUT_OF_PROCESS_SETTHREADCONTEXT
 
     // We pulse the GC mode here too cooperate w/ a thread trying to suspend the runtime. If we didn't pulse
     // the GC, the odds of catching this thread in interruptible code may be very small (since this filter
@@ -3312,43 +3270,6 @@ DPOSS_ACTION DebuggerController::DispatchPatchOrSingleStep(Thread *thread, CONTE
 
     }
 
-#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT
-    if (DebuggerController::GetProcessingDetach() && IsInUsedAction(used) && !isProcessingDetach)
-    {
-        int cDispatchedFlares = DebuggerController::IncrementDispatchedFlares();
-        isProcessingDetach = true;
-        LOG((LF_CORDB|LF_ENC, LL_INFO10000, "DC::DPOSS Incremented Flare Count = %d\n", cDispatchedFlares));
-    }
-
-    if (DebuggerController::GetProcessingDetach())
-    {
-        lockController.Acquire();
-
-        int cDispatchedExceptions = DebuggerController::DecrementActiveDispatchedExceptions();
-        LOG((LF_CORDB|LF_ENC, LL_INFO10000, "DC::DPOSS Decrement Exception Count = %d\n", cDispatchedExceptions));
-
-        int cDispatchedFlares = DebuggerController::GetDispatchedFlares();
-        _ASSERTE(cDispatchedFlares >= 0);
-        _ASSERTE(cDispatchedExceptions >= 0);
-
-        if (cDispatchedExceptions == 0)
-        {
-            DebuggerController::SetProcessingDetach(FALSE);
-            lockController.Release();
-
-            if (g_pDebugger != NULL)
-            {
-                g_pDebugger->SendDetachComplete();
-            }
-        }
-    }
-    else
-    {
-        int cDispatchedExceptions = DebuggerController::DecrementActiveDispatchedExceptions();
-        LOG((LF_CORDB|LF_ENC, LL_INFO10000, "DC::DPOSS Decrement Exception Count = %d\n", cDispatchedExceptions));
-    }
-#endif
-
     RETURN used;
 }
 
@@ -4463,26 +4384,43 @@ void DebuggerController::TriggerExternalMethodFixup(PCODE target)
 }
 
 #ifdef OUT_OF_PROCESS_SETTHREADCONTEXT
-/*static*/ int DebuggerController::GetNumPendingControllers()
+bool DebuggerController::CanSendDetach(bool fDecrementActiveExceptions)
 {
-    CONTRACTL
-    {
-        NOTHROW;
-        GC_NOTRIGGER;
-    }
-    CONTRACTL_END;
-    
-    ControllerLockHolder lockController;
-    int cTotalPendingControllers = 0;
-
-    for (DebuggerController* p = g_controllers; p != NULL; p = p->m_next)
+    bool fCanSendDetach = g_pDebugInterface == NULL || !g_pDebugInterface->IsOutOfProcessSetContextEnabled();
+    if (!fCanSendDetach)
     {
-        cTotalPendingControllers++;
+        ControllerLockHolder lockController;
+        bool fHasControllers = (g_controllers != NULL);
+        int cActiveDispatchedExceptions = fDecrementActiveExceptions ? DebuggerController::DecrementActiveDispatchedExceptions() : DebuggerController::GetActiveDispatchedExceptions();
+#ifdef _DEBUG
+        if (fDecrementActiveExceptions)
+        {
+            LOG((LF_CORDB, LL_INFO10000, "DC::CSD --active dispatched exceptions count = %d, HasControllers = %d\n", cActiveDispatchedExceptions, fHasControllers));
+        }
+        else
+        {
+            LOG((LF_CORDB, LL_INFO10000, "DC::CSD active dispatched exceptions count = %d, HasControllers = %d\n", cActiveDispatchedExceptions, fHasControllers));
+        }
+#endif
+        if (!fHasControllers && cActiveDispatchedExceptions == 0)
+        {
+            // No outstanding controllers or active dispatched exceptions, so it is safe to detach
+            DebuggerController::SetProcessingDetach(FALSE);
+            fCanSendDetach = true;
+            LOG((LF_CORDB, LL_INFO10000, "DC::CSD Can Detach\n"));
+        }
+        else if (!DebuggerController::GetProcessingDetach())
+        {
+            // this will cause DC::DNE to respond to this IPC once the queue of pending controllers has been processed
+            DebuggerController::SetProcessingDetach(TRUE);
+            fCanSendDetach = false;
+            LOG((LF_CORDB, LL_INFO10000, "DC::CSD Unable to Detach, need to drain existing controllers or exception processing\n"));
+        }
     }
 
-    return cTotalPendingControllers;
+    return fCanSendDetach;
 }
-#endif
+#endif // OUT_OF_PROCESS_SETTHREADCONTEXT
 
 #ifdef _DEBUG
 // See comment in DispatchNativeException
@@ -4583,16 +4521,6 @@ bool DebuggerController::DispatchNativeException(EXCEPTION_RECORD *pException,
         return FALSE;
     }
 
-    // It's possible we're here without a debugger (since we have to call the
-    // patch skippers). The Debugger may detach anytime,
-    // so remember the attach state now.
-#ifdef _DEBUG
-    bool fWasAttached = false;
-#ifdef DEBUGGING_SUPPORTED
-    fWasAttached = (CORDebuggerAttached() != 0);
-#endif //DEBUGGING_SUPPORTED
-#endif //_DEBUG
-
     {
         // If we're in cooperative mode, it's unsafe to do a GC until we've put a filter context in place.
         GCX_NOTRIGGER();
@@ -4632,6 +4560,15 @@ bool DebuggerController::DispatchNativeException(EXCEPTION_RECORD *pException,
         // Otherwise it is an error to nest at all
         _ASSERTE(pOldContext == NULL);
 
+        
+#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT
+        if (g_pDebugInterface != NULL && g_pDebugInterface->IsOutOfProcessSetContextEnabled())
+        {
+            int cActiveDispatchedExceptions = DebuggerController::IncrementActiveDispatchedExceptions();
+            LOG((LF_CORDB, LL_INFO10000, "DC::DNE ++active dispatched exceptions count = %d\n", cActiveDispatchedExceptions));
+        }
+#endif // OUT_OF_PROCESS_SETTHREADCONTEXT
+
         fDispatch = DebuggerController::DispatchExceptionHook(pCurThread,
                                                                    pContext,
                                                                    pException);
@@ -4695,13 +4632,6 @@ bool DebuggerController::DispatchNativeException(EXCEPTION_RECORD *pException,
 #endif
                                                        );
             LOG((LF_CORDB, LL_EVERYTHING, "DC::DNE DispatchPatch call returned\n"));
-
-            // If we detached, we should remove all our breakpoints. So if we try
-            // to handle this breakpoint, make sure that we're attached.
-            if (IsInUsedAction(result) == true)
-            {
-                _ASSERTE(fWasAttached);
-            }
             break;
 
         case EXCEPTION_SINGLE_STEP:
@@ -4759,6 +4689,31 @@ bool DebuggerController::DispatchNativeException(EXCEPTION_RECORD *pException,
         pCurThread->ApplySingleStep(pContext);
 #endif // FEATURE_EMULATE_SINGLESTEP
 
+#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT
+    {
+        if (fDebuggers && g_pDebugInterface != NULL && g_pDebugInterface->IsOutOfProcessSetContextEnabled())
+        {
+            int cDispatchedFlares = DebuggerController::IncrementDispatchedFlares();
+            LOG((LF_CORDB, LL_INFO10000, "DC::DNE ++dispatched flare count = %d\n", cDispatchedFlares));
+        }
+
+        if (DebuggerController::GetProcessingDetach())
+        {
+            LOG((LF_CORDB, LL_INFO10000, "DC::DNE, Processing Detach: calling CanSendDetach...\n"));
+            if (DebuggerController::CanSendDetach(true) && g_pDebugger != NULL)
+            {
+                LOG((LF_CORDB, LL_INFO10000, "DC::DNE, Detaching...\n"));
+                g_pDebugger->SendDetachComplete();
+            }
+        }
+        else if (g_pDebugInterface != NULL && g_pDebugInterface->IsOutOfProcessSetContextEnabled())
+        {
+            int cActiveDispatchedExceptions = DebuggerController::DecrementActiveDispatchedExceptions();
+            LOG((LF_CORDB, LL_INFO10000, "DC::DNE, NOT processing Detach, --active dispatched exceptions: %d\n", cActiveDispatchedExceptions));
+        }
+    }
+#endif // OUT_OF_PROCESS_SETTHREADCONTEXT
+
     FireEtwDebugExceptionProcessingEnd();
 
     return fDebuggers;
@@ -4916,7 +4871,7 @@ bool DebuggerPatchSkip::DebuggerDetachClean()
 
 #ifdef OUT_OF_PROCESS_SETTHREADCONTEXT
     // during detach we need to ensure the context is only being updated out of process
-    bool bUsingOutOfProcEvents = g_pDebugInterface->IsOutOfProcessSetContextEnabled();
+    bool bUsingOutOfProcEvents = g_pDebugInterface != NULL && g_pDebugInterface->IsOutOfProcessSetContextEnabled();
     if (bUsingOutOfProcEvents)
     {
         return false; // return false so that we do not delete this controller

src/coreclr/debug/ee/controller.h

@@ -1161,13 +1161,14 @@ class DebuggerController
     static bool GetProcessingDetach() {LIMITED_METHOD_CONTRACT;  return g_fProcessingDetach; }
     static void SetProcessingDetach(bool fProcessingDetach) {LIMITED_METHOD_CONTRACT;  g_fProcessingDetach = fProcessingDetach; }
     static int GetActiveDispatchedExceptions() {LIMITED_METHOD_CONTRACT;  return g_cActiveDispatchedExceptions; }
-    static void IncrementActiveDispatchedExceptions(int cActiveDispatchedExceptions) { LIMITED_METHOD_CONTRACT;  g_cActiveDispatchedExceptions += cActiveDispatchedExceptions; }
     static int IncrementActiveDispatchedExceptions() { LIMITED_METHOD_CONTRACT;  return (int)InterlockedIncrement(&g_cActiveDispatchedExceptions); }
     static int DecrementActiveDispatchedExceptions() { LIMITED_METHOD_CONTRACT;  return (int)InterlockedDecrement(&g_cActiveDispatchedExceptions); }
-    static int GetNumPendingControllers();
     static int GetDispatchedFlares() {LIMITED_METHOD_DAC_CONTRACT;  return g_cDispatchedFlares; }
-    static void SetDispatchedFlares(int cDispatchedFlares) { LIMITED_METHOD_DAC_CONTRACT; g_cDispatchedFlares = cDispatchedFlares; }
     static int IncrementDispatchedFlares() { LIMITED_METHOD_DAC_CONTRACT;  return (int)InterlockedIncrement(&g_cDispatchedFlares); }
+    static void ResetDispatchedFlares() { LIMITED_METHOD_DAC_CONTRACT;  g_cDispatchedFlares = 0; }
+    static bool CanSendDetach(bool fDecrementActiveExceptions = false);
+#else
+    static int GetDispatchedFlares() {LIMITED_METHOD_DAC_CONTRACT;  return 0; }
 #endif
 
 #if defined(_DEBUG)
@@ -1224,23 +1225,19 @@ class DebuggerController
     static int g_cTotalMethodEnter;
 
     // When detach is initiated, the runtime is synchronized.
-    // This is a running count of dispatched exceptions that
-    // could result in a SetThreadContextNeeded event.
+    // However, it is possible that DC::DNE is still processing debugger events.
+    // This is a running count of exceptions running inside of DC::DNE
     // Detach will use this to get an accurate count of
     // SetThreadContextNeeded flares after continuing.
-    // State is tracked globally because the controllers are deleted
-    // during the detach operation
-    // This variable is only used while the debugger is attached
     static Volatile<DWORD> g_cActiveDispatchedExceptions;
 
     // During detach we need to wait until all flares have been processed
     // before allowing the detach to proceed. Once this flag is set
     // true, we will start tracking the number of dispatched flares.
     static Volatile<BOOL> g_fProcessingDetach;
 
-    // This is the number of flares that have been sent after 
-    // continuing inside of a detach request 
-    // This will be sent to the right side to ensure that it stays attached
+    // This is the number of flares that have been sent 
+    // It will be sent to the right side to ensure that it stays attached
     // long enough to process any SetThreadContextNeeded events
     static Volatile<DWORD> g_cDispatchedFlares;