Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Porting SuspendAllThreads from the NativeAOT to CoreCLR. #101782

Merged
merged 26 commits into from
May 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions docs/design/datacontracts/Thread.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/debug/ee/debugger.h
Original file line number Diff line number Diff line change
Expand Up @@ -3807,7 +3807,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(); \
Expand Down
8 changes: 0 additions & 8 deletions src/coreclr/nativeaot/Runtime/thread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -328,14 +328,6 @@ bool Thread::IsGCSpecial()
return IsStateSet(TSF_IsGcSpecialThread);
}

bool Thread::CatchAtSafePoint()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was dead code already. Noone called this.

{
// 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;
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/nativeaot/Runtime/thread.h
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,6 @@ class Thread : private ThreadBuffer
//
void SetGCSpecial();
bool IsGCSpecial();
bool CatchAtSafePoint();

//
// Managed/unmanaged interop transitions support APIs
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/pal/inc/pal.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/pal/src/exception/signal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 1 addition & 3 deletions src/coreclr/vm/comtoclrcall.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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()
};
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/vm/eedbginterfaceimpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,7 @@ void EEDbgInterfaceImpl::EnablePreemptiveGC(void)
CONTRACTL
{
NOTHROW;
DISABLED(GC_TRIGGERS); // Disabled because disabled in RareEnablePreemptiveGC()
GC_NOTRIGGER;
}
CONTRACTL_END;

Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/vm/fcall.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/vm/fcall.h
Original file line number Diff line number Diff line change
Expand Up @@ -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; \
Expand All @@ -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 */ \
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/vm/frames.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
25 changes: 0 additions & 25 deletions src/coreclr/vm/i386/asmhelpers.S
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
24 changes: 0 additions & 24 deletions src/coreclr/vm/i386/asmhelpers.asm
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -394,29 +393,6 @@ endif
retn 8
_CallJitEHFinallyHelper@8 ENDP

;-----------------------------------------------------------------------
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We stopped doing thread trapping on transitions from coop in JIT helpers a while ago.
It looks like x86 stubs were left behind and were still doing it.

; 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
Expand Down
12 changes: 0 additions & 12 deletions src/coreclr/vm/i386/cgenx86.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
Loading
Loading