From 37c185dce58986ffc6951c6464d5d126fdfadd2d Mon Sep 17 00:00:00 2001
From: Andrew Au <andrewau@microsoft.com>
Date: Fri, 15 Jun 2018 16:18:48 -0700
Subject: [PATCH] Properly fix the DB_IPCE_ASYNC_BREAK case

Commit migrated from https://github.com/dotnet/coreclr/commit/8a7ab918952d0cb06ef57e2cf5c255f237e49822
---
 src/coreclr/src/debug/ee/debugger.cpp | 28 ++++++++--------------------
 src/coreclr/src/debug/ee/debugger.h   | 21 +++++++++++----------
 2 files changed, 19 insertions(+), 30 deletions(-)

diff --git a/src/coreclr/src/debug/ee/debugger.cpp b/src/coreclr/src/debug/ee/debugger.cpp
index 8439e0f..d029750 100644
--- a/src/coreclr/src/debug/ee/debugger.cpp
+++ b/src/coreclr/src/debug/ee/debugger.cpp
@@ -5506,13 +5506,7 @@ void Debugger::TrapAllRuntimeThreads()
         m_trappingRuntimeThreads = TRUE;
 
         // Take the thread store lock.
-        bool alreadyHeldThreadStoreLock = ThreadStore::HoldingThreadStore();
-        if (!alreadyHeldThreadStoreLock)
-        {
-            STRESS_LOG0(LF_CORDB, LL_INFO1000, "About to lock thread Store\n");
-            ThreadSuspend::LockThreadStore(ThreadSuspend::SUSPEND_FOR_DEBUGGER);
-            STRESS_LOG0(LF_CORDB, LL_INFO1000, "Locked thread store\n");
-        }
+        assert(ThreadStore::HoldingThreadStore());
 
         // We start the suspension here, and let the helper thread finish it.
         // If there's no helper thread, then we need to do helper duty.
@@ -5555,19 +5549,6 @@ void Debugger::TrapAllRuntimeThreads()
 
             // We will have released the TSL after the call to continue.
         }
-        else
-        {
-            // We have a live and active helper thread which will handle events
-            // from the RS now that we're stopped.
-            // We need to release the TSL which we acquired above. (The helper will
-            // likely take this lock while doing stuff).
-            if (!alreadyHeldThreadStoreLock)
-            {
-                STRESS_LOG0(LF_CORDB, LL_INFO1000, "About to unlock thread store!\n");
-                ThreadSuspend::UnlockThreadStore(FALSE, ThreadSuspend::SUSPEND_FOR_DEBUGGER);
-                STRESS_LOG0(LF_CORDB, LL_INFO1000, "TART: Unlocked thread store!\n");
-            }
-        }
         _ASSERTE(ThreadHoldsLock()); // still hold the lock. (though it may have been toggled)
     }
 }
@@ -10758,11 +10739,14 @@ bool Debugger::HandleIPCEvent(DebuggerIPCEvent * pEvent)
     // be sent as part of attach if debugger was launched by managed app.
     //
     DebuggerLockHolder dbgLockHolder(this, FALSE);
+    bool lockedThreadStore = false;
 
     if ((pEvent->type & DB_IPCE_TYPE_MASK) == DB_IPCE_ASYNC_BREAK ||
         (pEvent->type & DB_IPCE_TYPE_MASK) == DB_IPCE_ATTACHING ||
         this->m_isBlockedOnGarbageCollectionEvent)
     {
+        lockedThreadStore = true;
+        ThreadSuspend::LockThreadStore(ThreadSuspend::SUSPEND_FOR_DEBUGGER);
         dbgLockHolder.Acquire();
     }
     else
@@ -11767,6 +11751,10 @@ bool Debugger::HandleIPCEvent(DebuggerIPCEvent * pEvent)
 
     STRESS_LOG0(LF_CORDB, LL_INFO10000, "D::HIPCE: finished handling event\n");
 
+    if (lockedThreadStore)
+    {
+        ThreadSuspend::UnlockThreadStore(FALSE, ThreadSuspend::SUSPEND_FOR_DEBUGGER);
+    }
     // dbgLockHolder goes out of scope - implicit Release
     return fContinue;
 }
diff --git a/src/coreclr/src/debug/ee/debugger.h b/src/coreclr/src/debug/ee/debugger.h
index 10d62d6..a342c7a 100644
--- a/src/coreclr/src/debug/ee/debugger.h
+++ b/src/coreclr/src/debug/ee/debugger.h
@@ -32,6 +32,7 @@
 #include "common.h"
 #include "winwrap.h"
 #include "threads.h"
+#include "threadsuspend.h"
 #include "frames.h"
 
 #include "appdomain.hpp"
@@ -2959,7 +2960,7 @@ public:
 #endif
 private:
     HANDLE GetGarbageCollectionBlockerEvent() { return  GetLazyData()->m_garbageCollectionBlockerEvent; }
-    
+
 };
 
 
@@ -3794,16 +3795,16 @@ HANDLE OpenWin32EventOrThrow(
     LPCWSTR lpName
 );
 
-#define SENDIPCEVENT_RAW_BEGIN_EX(pDbgLockHolder, gcxStmt)      \
-  {                                                             \
-    ThreadStore::LockThreadStore();                             \
-    Debugger::DebuggerLockHolder *__pDbgLockHolder = pDbgLockHolder; \
-    gcxStmt;                                                    \
+#define SENDIPCEVENT_RAW_BEGIN_EX(pDbgLockHolder, gcxStmt)               \
+  {                                                                      \
+    ThreadSuspend::LockThreadStore(ThreadSuspend::SUSPEND_FOR_DEBUGGER); \
+    Debugger::DebuggerLockHolder *__pDbgLockHolder = pDbgLockHolder;     \
+    gcxStmt;                                                             \
     g_pDebugger->LockForEventSending(__pDbgLockHolder);
 
-#define SENDIPCEVENT_RAW_END_EX                                 \
-    g_pDebugger->UnlockFromEventSending(__pDbgLockHolder);      \
-    ThreadStore::UnlockThreadStore();                             \
+#define SENDIPCEVENT_RAW_END_EX                                          \
+    g_pDebugger->UnlockFromEventSending(__pDbgLockHolder);               \
+    ThreadStore::UnlockThreadStore();                                    \
   }
 
 #define SENDIPCEVENT_RAW_BEGIN(pDbgLockHolder)                  \
@@ -3828,7 +3829,7 @@ HANDLE OpenWin32EventOrThrow(
         Debugger::DebuggerLockHolder __dbgLockHolder(pDebugger, FALSE);                   \
         Debugger::DebuggerLockHolder *__pDbgLockHolder = &__dbgLockHolder;                \
         gcxStmt;                                                                          \
-        ThreadStore::LockThreadStore();                                                   \
+        ThreadSuspend::LockThreadStore(ThreadSuspend::SUSPEND_FOR_DEBUGGER);              \
         g_pDebugger->LockForEventSending(__pDbgLockHolder);                               \
         /* Check if the thread has been suspended by the debugger via SetDebugState(). */ \
         if (thread != NULL && thread->HasThreadStateNC(Thread::TSNC_DebuggerUserSuspend)) \
-- 
2.7.4