Revert changes relevant to forbid-suspend-for-debugger scopes

Author: Koundinya Veluri

src/coreclr/vm/appdomain.hpp

@@ -1177,8 +1177,7 @@ class BaseDomain
 
     // To be used when the thread may enter cooperative GC mode while holding the lock. The thread enters a
     // forbid-suspend-for-debugger region along with acquiring the lock, such that it would not suspend for the debugger while
-    // holding the lock, as that may otherwise cause a FuncEval to deadlock when trying to acquire the lock. This holder cannot
-    // be used when there may be GC allocation inside its scope (see the base class for more information).
+    // holding the lock, as that may otherwise cause a FuncEval to deadlock when trying to acquire the lock.
     class DomainCacheCrstHolderForGCCoop : private CrstAndForbidSuspendForDebuggerHolder
     {
     public:

src/coreclr/vm/crst.h

@@ -395,16 +395,6 @@ friend class Crst;
     // This just exists to convert legacy OS Critical Section patterns over to holders.
     typedef DacHolder<CrstBase *, CrstBase::ReleaseLock, CrstBase::AcquireLock, 0, CompareDefault> UnsafeCrstInverseHolder;
 
-    // This holder acquires the lock in preemptive GC mode and puts the thread in a forbid-suspend-for-debugger region. It is
-    // used when cooperative GC mode would be entered while the lock is held, which can normally suspend for the debugger and
-    // cause FuncEvals to deadlock upon trying to acquire the lock. Using this holder prevents the thread from suspending for
-    // the debugger upon GC mode switches inside the forbid region.
-    //
-    // However, the forbid region does not prevent the thread from waiting for a pending GC upon allocating from the GC. For
-    // instance, some other thread may have started a GC and suspended for the debugger, then this thread allocates from the GC
-    // and needs to perform a GC, so it waits for the other GC to complete. At that point debugger suspension cannot complete
-    // because this thread is stuck inside the forbid region. So, this holder should not be used in cases that may allocate from
-    // the GC.
     class CrstAndForbidSuspendForDebuggerHolder
     {
     private:

src/coreclr/vm/threadsuspend.cpp

@@ -2113,18 +2113,9 @@ void Thread::RareDisablePreemptiveGC()
     // 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
-    if ((
-            (GCHeapUtilities::IsGCInProgress() && (this != ThreadSuspend::GetSuspensionThread())) ||
-            (m_State & TS_DebugSuspendPending) ||
-            (m_State & TS_StackCrawlNeeded)
-        ) &&
-
-        // When in a forbid-suspend-for-debugger region and the thread waits for an in-progress GC, it would become stuck in the
-        // forbid region until the GC is complete. In the meantime, the debugger may need to have the runtime suspend for the
-        // debugger first, perhaps in a GC-unsafe point. This thread would not be able to suspend for the debugger and would
-        // cause a deadlock during debugger suspension. When in the forbid region, allow disabling preemptive GC and bypass the
-        // above heuristics.
-        !IsInForbidSuspendForDebuggerRegion())
+    if ((GCHeapUtilities::IsGCInProgress() && (this != ThreadSuspend::GetSuspensionThread())) ||
+        ((m_State & TS_DebugSuspendPending) && !IsInForbidSuspendForDebuggerRegion()) ||
+        (m_State & TS_StackCrawlNeeded))
     {
         STRESS_LOG1(LF_SYNC, LL_INFO1000, "RareDisablePreemptiveGC: entering. Thread state = %x\n", m_State.Load());
 
@@ -2190,10 +2181,9 @@ void Thread::RareDisablePreemptiveGC()
             // However, it is possible for the current thread to become the GC
             // thread while in this loop.  This happens if you use the COM+
             // debugger to suspend this thread and then release it.
-            _ASSERTE(!IsInForbidSuspendForDebuggerRegion());
             if (! ((GCHeapUtilities::IsGCInProgress() && (this != ThreadSuspend::GetSuspensionThread())) ||
-                   (m_State & TS_DebugSuspendPending) ||
-                   (m_State & TS_StackCrawlNeeded)) )
+                    ((m_State & TS_DebugSuspendPending) && !IsInForbidSuspendForDebuggerRegion()) ||
+                    (m_State & TS_StackCrawlNeeded)) )
             {
                 break;
             }