Porting SuspendAllThreads from the NativeAOT to CoreCLR. (dotnet#10…

…1782)

* port suspension algo from NativeAOT

* PING_JIT_TIMEOUT gone

* CatchAtSafePoint is always opportunistic

* current

* removed RareEnablePreemptiveGC

* cleanup RareDisablePreemptiveGC

* fix for x86

* factored out Thread::Hijack

* fix build for non-x64 windows

* assert noone holds TSL into coop mode

* activation safety check is always for the current thread

* undo comment

* PulseGCMode should not check for CatchAtSafePointOpportunistic

* not disabling preempt while holding TSL

* tweak

* dead assert

* tweak RareDisablePreemptiveGC

* RareDisablePreemptiveGC avoid GetSuspensionThread()

* updated Thread::Hijack

* fix typo

* allow coop mode while holding TS lock

* Refactored into SuspendAllThreads/ResumeAllThreads

* SetThreadTrapForSuspension

* deleted TS_GCSuspendPending

* tweaks

* PR feedback

docs/design/datacontracts/Thread.md

@@ -22,9 +22,7 @@ enum ThreadState
 
     TS_AbortRequested         = 0x00000001,    // Abort the thread
 
-    TS_GCSuspendPending       = 0x00000002,    // ThreadSuspend::SuspendRuntime watches this thread to leave coop mode.
     TS_GCSuspendRedirected    = 0x00000004,    // ThreadSuspend::SuspendRuntime has redirected the thread to suspention routine.
-    TS_GCSuspendFlags         = TS_GCSuspendPending | TS_GCSuspendRedirected, // used to track suspension progress. Only SuspendRuntime writes/resets these.
 
     TS_DebugSuspendPending    = 0x00000008,    // Is the debugger suspending threads?
     TS_GCOnTransitions        = 0x00000010,    // Force a GC on stub transitions (GCStress only)

src/coreclr/debug/ee/debugger.h

@@ -3893,7 +3893,7 @@ HANDLE OpenWin32EventOrThrow(
 // debugger may not be expecting it.  Instead, just leave the lock and retry.
 // When we leave, we'll enter coop mode first and get suspended if a suspension is in progress.
 // Afterwards, we'll transition back into preemptive mode, and we'll block because this thread
-// has been suspended by the debugger (see code:Thread::RareEnablePreemptiveGC).
+// has been suspended by the debugger (see code:Thread::RareDisablePreemptiveGC).
 #define SENDIPCEVENT_BEGIN_EX(pDebugger, thread, gcxStmt)                                 \
   {                                                                                       \
     FireEtwDebugIPCEventStart();                                                          \

src/coreclr/nativeaot/Runtime/thread.cpp

@@ -328,14 +328,6 @@ bool Thread::IsGCSpecial()
     return IsStateSet(TSF_IsGcSpecialThread);
 }
 
-bool Thread::CatchAtSafePoint()
-{
-    // This is only called by the GC on a background GC worker thread that's explicitly interested in letting
-    // a foreground GC proceed at that point. So it's always safe to return true.
-    ASSERT(IsGCSpecial());
-    return true;
-}
-
 uint64_t Thread::GetPalThreadIdForLogging()
 {
     return m_threadId;

src/coreclr/nativeaot/Runtime/thread.h

@@ -297,7 +297,6 @@ class Thread : private ThreadBuffer
     //
     void SetGCSpecial();
     bool IsGCSpecial();
-    bool CatchAtSafePoint();
 
     //
     // Managed/unmanaged interop transitions support APIs

src/coreclr/pal/inc/pal.h

@@ -3772,7 +3772,7 @@ PALAPI
 FlushProcessWriteBuffers();
 
 typedef void (*PAL_ActivationFunction)(CONTEXT *context);
-typedef BOOL (*PAL_SafeActivationCheckFunction)(SIZE_T ip, BOOL checkingCurrentThread);
+typedef BOOL (*PAL_SafeActivationCheckFunction)(SIZE_T ip);
 
 PALIMPORT
 VOID

src/coreclr/pal/src/exception/signal.cpp

@@ -833,7 +833,7 @@ static void inject_activation_handler(int code, siginfo_t *siginfo, void *contex
             &winContext,
             contextFlags);
 
-        if (g_safeActivationCheckFunction(CONTEXTGetPC(&winContext), /* checkingCurrentThread */ TRUE))
+        if (g_safeActivationCheckFunction(CONTEXTGetPC(&winContext)))
         {
             g_inject_activation_context_locvar_offset = (int)((char*)&winContext - (char*)__builtin_frame_address(0));
             int savedErrNo = errno; // Make sure that errno is not modified

src/coreclr/vm/comtoclrcall.cpp

@@ -515,7 +515,7 @@ extern "C" UINT64 __stdcall COMToCLRWorker(Thread *pThread, ComMethodFrame* pFra
     // we have additional checks for thread abort that are performed only when
     // g_TrapReturningThreads is set.
     pThread->m_fPreemptiveGCDisabled.StoreWithoutBarrier(1);
-    if (g_TrapReturningThreads.LoadWithoutBarrier())
+    if (g_TrapReturningThreads)
     {
         hr = StubRareDisableHRWorker(pThread);
         if (S_OK != hr)
@@ -1339,14 +1339,12 @@ Stub* ComCall::CreateGenericComCallStub(BOOL isFieldAccess)
     //  ends up throwing an OOM exception.
 
     CodeLabel* rgRareLabels[] = {
-                                  psl->NewCodeLabel(),
                                   psl->NewCodeLabel(),
                                   psl->NewCodeLabel()
                                 };
 
 
     CodeLabel* rgRejoinLabels[] = {
-                                    psl->NewCodeLabel(),
                                     psl->NewCodeLabel(),
                                     psl->NewCodeLabel()
                                   };

src/coreclr/vm/eedbginterfaceimpl.cpp

@@ -652,7 +652,7 @@ void EEDbgInterfaceImpl::EnablePreemptiveGC(void)
     CONTRACTL
     {
         NOTHROW;
-        DISABLED(GC_TRIGGERS); // Disabled because disabled in RareEnablePreemptiveGC()
+        GC_NOTRIGGER;
     }
     CONTRACTL_END;
 

src/coreclr/vm/fcall.cpp

@@ -129,7 +129,7 @@ NOINLINE Object* FC_GCPoll(void* __me, Object* objToProtect)
     INCONTRACT(FCallCheck __fCallCheck(__FILE__, __LINE__));
 
     Thread  *thread = GetThread();
-    if (thread->CatchAtSafePointOpportunistic())    // Does someone want this thread stopped?
+    if (thread->CatchAtSafePoint())    // Does someone want this thread stopped?
     {
         HELPER_METHOD_FRAME_BEGIN_RET_ATTRIB_1(Frame::FRAME_ATTR_CAPTURE_DEPTH_2, objToProtect);
 

src/coreclr/vm/fcall.h

@@ -809,7 +809,7 @@ Object* FC_GCPoll(void* me, Object* objToProtect = NULL);
     {                                                       \
         INCONTRACT(Thread::TriggersGC(GetThread());)        \
         INCONTRACT(__fCallCheck.SetDidPoll();)              \
-        if (g_TrapReturningThreads.LoadWithoutBarrier())    \
+        if (g_TrapReturningThreads)    \
         {                                                   \
             if (FC_GCPoll(__me))                            \
                 return ret;                                 \
@@ -824,7 +824,7 @@ Object* FC_GCPoll(void* me, Object* objToProtect = NULL);
     {                                                       \
         INCONTRACT(__fCallCheck.SetDidPoll();)              \
         Object* __temp = OBJECTREFToObject(obj);            \
-        if (g_TrapReturningThreads.LoadWithoutBarrier())    \
+        if (g_TrapReturningThreads)    \
         {                                                   \
             __temp = FC_GCPoll(__me, __temp);               \
             while (0 == FC_NO_TAILCALL) { }; /* side effect the compile can't remove */  \

src/coreclr/vm/frames.h

@@ -1332,7 +1332,7 @@ class HelperMethodFrame : public Frame
     FORCEINLINE void Poll()
     {
         WRAPPER_NO_CONTRACT;
-        if (m_pThread->CatchAtSafePointOpportunistic())
+        if (m_pThread->CatchAtSafePoint())
             CommonTripThread();
     }
 #endif // DACCESS_COMPILE

src/coreclr/vm/i386/asmhelpers.S

@@ -126,31 +126,6 @@ LEAF_ENTRY RestoreFPUContext, _TEXT
     ret     4
 LEAF_END RestoreFPUContext, _TEXT
 
-// -----------------------------------------------------------------------
-//  The out-of-line portion of the code to enable preemptive GC.
-//  After the work is done, the code jumps back to the "pRejoinPoint"
-//  which should be emitted right after the inline part is generated.
-//
-//  Assumptions:
-//       ebx = Thread
-//  Preserves
-//       all registers except ecx.
-//
-// -----------------------------------------------------------------------
-NESTED_ENTRY StubRareEnable, _TEXT, NoHandler
-    push    eax
-    push    edx
-
-    push    ebx
-
-    CHECK_STACK_ALIGNMENT
-    call    C_FUNC(StubRareEnableWorker)
-
-    pop     edx
-    pop     eax
-    ret
-NESTED_END StubRareEnable, _TEXT
-
 NESTED_ENTRY StubRareDisableTHROW, _TEXT, NoHandler
     push    eax
     push    edx

src/coreclr/vm/i386/asmhelpers.asm

@@ -24,7 +24,6 @@ EXTERN __imp__RtlUnwind@16:DWORD
 ifdef _DEBUG
 EXTERN _HelperMethodFrameConfirmState@20:PROC
 endif
-EXTERN _StubRareEnableWorker@4:PROC
 ifdef FEATURE_COMINTEROP
 EXTERN _StubRareDisableHRWorker@4:PROC
 endif ; FEATURE_COMINTEROP
@@ -394,29 +393,6 @@ endif
         retn    8
 _CallJitEHFinallyHelper@8 ENDP
 
-;-----------------------------------------------------------------------
-; The out-of-line portion of the code to enable preemptive GC.
-; After the work is done, the code jumps back to the "pRejoinPoint"
-; which should be emitted right after the inline part is generated.
-;
-; Assumptions:
-;      ebx = Thread
-; Preserves
-;      all registers except ecx.
-;
-;-----------------------------------------------------------------------
-_StubRareEnable proc public
-        push    eax
-        push    edx
-
-        push    ebx
-        call    _StubRareEnableWorker@4
-
-        pop     edx
-        pop     eax
-        retn
-_StubRareEnable ENDP
-
 ifdef FEATURE_COMINTEROP
 _StubRareDisableHR proc public
         push    edx

src/coreclr/vm/i386/cgenx86.cpp

@@ -911,18 +911,6 @@ Stub *GenerateInitPInvokeFrameHelper()
     RETURN psl->Link(SystemDomain::GetGlobalLoaderAllocator()->GetExecutableHeap());
 }
 
-
-extern "C" VOID STDCALL StubRareEnableWorker(Thread *pThread)
-{
-    WRAPPER_NO_CONTRACT;
-
-    //printf("RareEnable\n");
-    pThread->RareEnablePreemptiveGC();
-}
-
-
-
-
 // Disable when calling into managed code from a place that fails via Exceptions
 extern "C" VOID STDCALL StubRareDisableTHROWWorker(Thread *pThread)
 {