diff --git a/src/coreclr/debug/ee/debugger.h b/src/coreclr/debug/ee/debugger.h index 43f56c00dc793..9e03cdf506c00 100644 --- a/src/coreclr/debug/ee/debugger.h +++ b/src/coreclr/debug/ee/debugger.h @@ -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(); \ diff --git a/src/coreclr/vm/eedbginterfaceimpl.cpp b/src/coreclr/vm/eedbginterfaceimpl.cpp index 4634a3a3cc5a3..822a46a536e39 100644 --- a/src/coreclr/vm/eedbginterfaceimpl.cpp +++ b/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; diff --git a/src/coreclr/vm/i386/asmhelpers.S b/src/coreclr/vm/i386/asmhelpers.S index ff777550bfab5..54f58fd75bb62 100644 --- a/src/coreclr/vm/i386/asmhelpers.S +++ b/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 diff --git a/src/coreclr/vm/i386/asmhelpers.asm b/src/coreclr/vm/i386/asmhelpers.asm index 1d02fc48f8d8b..5f03683f588fa 100644 --- a/src/coreclr/vm/i386/asmhelpers.asm +++ b/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 diff --git a/src/coreclr/vm/i386/cgenx86.cpp b/src/coreclr/vm/i386/cgenx86.cpp index 108bc66a99b15..d9dc22d148193 100644 --- a/src/coreclr/vm/i386/cgenx86.cpp +++ b/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) { diff --git a/src/coreclr/vm/i386/stublinkerx86.cpp b/src/coreclr/vm/i386/stublinkerx86.cpp index 76d888c0c5275..7dde57bf8ebd5 100644 --- a/src/coreclr/vm/i386/stublinkerx86.cpp +++ b/src/coreclr/vm/i386/stublinkerx86.cpp @@ -45,8 +45,6 @@ #ifndef DACCESS_COMPILE - -extern "C" VOID __cdecl StubRareEnable(Thread *pThread); #ifdef FEATURE_COMINTEROP extern "C" HRESULT __cdecl StubRareDisableHR(Thread *pThread); #endif // FEATURE_COMINTEROP @@ -2615,6 +2613,8 @@ void StubLinkerCPU::EmitComMethodStubEpilog(TADDR pFrameVptr, PRECONDITION(rgRareLabels[0] != NULL && rgRareLabels[1] != NULL && rgRareLabels[2] != NULL); PRECONDITION(rgRejoinLabels != NULL); PRECONDITION(rgRejoinLabels[0] != NULL && rgRejoinLabels[1] != NULL && rgRejoinLabels[2] != NULL); + PRECONDITION(4 == sizeof( ((Thread*)0)->m_State )); + PRECONDITION(4 == sizeof( ((Thread*)0)->m_fPreemptiveGCDisabled )); } CONTRACTL_END; @@ -2623,11 +2623,9 @@ void StubLinkerCPU::EmitComMethodStubEpilog(TADDR pFrameVptr, // mov [ebx + Thread.GetFrame()], edi ;; restore previous frame X86EmitIndexRegStore(kEBX, Thread::GetOffsetOfCurrentFrame(), kEDI); - //----------------------------------------------------------------------- - // Generate the inline part of disabling preemptive GC - //----------------------------------------------------------------------- - EmitEnable(rgRareLabels[2]); // rare gc - EmitLabel(rgRejoinLabels[2]); // rejoin for rare gc + // move byte ptr [ebx + Thread.m_fPreemptiveGCDisabled],0 + X86EmitOffsetModRM(0xc6, (X86Reg)0, kEBX, Thread::GetOffsetOfGCFlag()); + Emit8(0); // add esp, popstack X86EmitAddEsp(sizeof(GSCookie) + UnmanagedToManagedFrame::GetOffsetOfCalleeSavedRegisters()); @@ -2651,12 +2649,6 @@ void StubLinkerCPU::EmitComMethodStubEpilog(TADDR pFrameVptr, // keeps on going past the previous "jmp eax". X86EmitReturn(0); - //----------------------------------------------------------------------- - // The out-of-line portion of enabling preemptive GC - rarely executed - //----------------------------------------------------------------------- - EmitLabel(rgRareLabels[2]); // label for rare enable gc - EmitRareEnable(rgRejoinLabels[2]); // emit rare enable gc - //----------------------------------------------------------------------- // The out-of-line portion of disabling preemptive GC - rarely executed //----------------------------------------------------------------------- @@ -2736,6 +2728,8 @@ void StubLinkerCPU::EmitSharedComMethodStubEpilog(TADDR pFrameVptr, PRECONDITION(rgRareLabels[0] != NULL && rgRareLabels[1] != NULL && rgRareLabels[2] != NULL); PRECONDITION(rgRejoinLabels != NULL); PRECONDITION(rgRejoinLabels[0] != NULL && rgRejoinLabels[1] != NULL && rgRejoinLabels[2] != NULL); + PRECONDITION(4 == sizeof( ((Thread*)0)->m_State )); + PRECONDITION(4 == sizeof( ((Thread*)0)->m_fPreemptiveGCDisabled )); } CONTRACTL_END; @@ -2748,12 +2742,13 @@ void StubLinkerCPU::EmitSharedComMethodStubEpilog(TADDR pFrameVptr, X86EmitIndexRegStore(kEBX, Thread::GetOffsetOfCurrentFrame(), kEDI); //----------------------------------------------------------------------- - // Generate the inline part of enabling preemptive GC + // Generate enabling preemptive GC //----------------------------------------------------------------------- EmitLabel(NoEntryLabel); // need to enable preemp mode even when we fail the disable as rare disable will return in coop mode - EmitEnable(rgRareLabels[2]); // rare enable gc - EmitLabel(rgRejoinLabels[2]); // rejoin for rare enable gc + // move byte ptr [ebx + Thread.m_fPreemptiveGCDisabled],0 + X86EmitOffsetModRM(0xc6, (X86Reg)0, kEBX, Thread::GetOffsetOfGCFlag()); + Emit8(0); #ifdef PROFILING_SUPPORTED // If profiling is active, emit code to notify profiler of transition @@ -2800,12 +2795,6 @@ void StubLinkerCPU::EmitSharedComMethodStubEpilog(TADDR pFrameVptr, // keeps on going past the previous "jmp ecx". X86EmitReturn(0); - //----------------------------------------------------------------------- - // The out-of-line portion of enabling preemptive GC - rarely executed - //----------------------------------------------------------------------- - EmitLabel(rgRareLabels[2]); // label for rare enable gc - EmitRareEnable(rgRejoinLabels[2]); // emit rare enable gc - //----------------------------------------------------------------------- // The out-of-line portion of disabling preemptive GC - rarely executed //----------------------------------------------------------------------- @@ -3335,77 +3324,6 @@ VOID StubLinkerCPU::EmitUnwindInfoCheckSubfunction() #if defined(FEATURE_COMINTEROP) && defined(TARGET_X86) -//----------------------------------------------------------------------- -// Generates the inline portion of the code to enable preemptive GC. Hopefully, -// the inline code is all that will execute most of the time. If this code -// path is entered at certain times, however, it will need to jump out to -// a separate out-of-line path which is more expensive. The "pForwardRef" -// label indicates the start of the out-of-line path. -// -// Assumptions: -// ebx = Thread -// Preserves -// all registers except ecx. -// -//----------------------------------------------------------------------- -VOID StubLinkerCPU::EmitEnable(CodeLabel *pForwardRef) -{ - CONTRACTL - { - STANDARD_VM_CHECK; - - PRECONDITION(4 == sizeof( ((Thread*)0)->m_State )); - PRECONDITION(4 == sizeof( ((Thread*)0)->m_fPreemptiveGCDisabled )); - } - CONTRACTL_END; - - // move byte ptr [ebx + Thread.m_fPreemptiveGCDisabled],0 - X86EmitOffsetModRM(0xc6, (X86Reg)0, kEBX, Thread::GetOffsetOfGCFlag()); - Emit8(0); - - _ASSERTE(FitsInI1(Thread::TS_CatchAtSafePoint)); - - // test byte ptr [ebx + Thread.m_State], TS_CatchAtSafePoint - X86EmitOffsetModRM(0xf6, (X86Reg)0, kEBX, Thread::GetOffsetOfState()); - Emit8(Thread::TS_CatchAtSafePoint); - - // jnz RarePath - X86EmitCondJump(pForwardRef, X86CondCode::kJNZ); - -#ifdef _DEBUG - X86EmitDebugTrashReg(kECX); -#endif - -} - - -//----------------------------------------------------------------------- -// Generates 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. -// -//----------------------------------------------------------------------- -VOID StubLinkerCPU::EmitRareEnable(CodeLabel *pRejoinPoint) -{ - STANDARD_VM_CONTRACT; - - X86EmitCall(NewExternalCodeLabel((LPVOID) StubRareEnable), 0); -#ifdef _DEBUG - X86EmitDebugTrashReg(kECX); -#endif - if (pRejoinPoint) - { - X86EmitNearJump(pRejoinPoint); - } - -} - - //----------------------------------------------------------------------- // Generates the inline portion of the code to disable preemptive GC. Hopefully, // the inline code is all that will execute most of the time. If this code diff --git a/src/coreclr/vm/i386/stublinkerx86.h b/src/coreclr/vm/i386/stublinkerx86.h index 922babee24a2f..7afb4f6a5ffe8 100644 --- a/src/coreclr/vm/i386/stublinkerx86.h +++ b/src/coreclr/vm/i386/stublinkerx86.h @@ -323,9 +323,6 @@ class StubLinkerCPU : public StubLinker } #if defined(FEATURE_COMINTEROP) && defined(TARGET_X86) - VOID EmitEnable(CodeLabel *pForwardRef); - VOID EmitRareEnable(CodeLabel *pRejoinPoint); - VOID EmitDisable(CodeLabel *pForwardRef, BOOL fCallIn, X86Reg ThreadReg); VOID EmitRareDisable(CodeLabel *pRejoinPoint); VOID EmitRareDisableHRESULT(CodeLabel *pRejoinPoint, CodeLabel *pExitPoint); diff --git a/src/coreclr/vm/threads.cpp b/src/coreclr/vm/threads.cpp index 1c9d27a6e2aab..34b90e0647e91 100644 --- a/src/coreclr/vm/threads.cpp +++ b/src/coreclr/vm/threads.cpp @@ -1399,13 +1399,6 @@ Thread::Thread() #ifdef _DEBUG m_ulForbidTypeLoad = 0; m_GCOnTransitionsOK = TRUE; -#endif - -#ifdef ENABLE_CONTRACTS - m_ulEnablePreemptiveGCCount = 0; -#endif - -#ifdef _DEBUG dbg_m_cSuspendedThreads = 0; dbg_m_cSuspendedThreadsWithoutOSLock = 0; m_Creator.Clear(); diff --git a/src/coreclr/vm/threads.h b/src/coreclr/vm/threads.h index 2e1b979ed1ed1..cbd795ff99886 100644 --- a/src/coreclr/vm/threads.h +++ b/src/coreclr/vm/threads.h @@ -1397,11 +1397,6 @@ class Thread // holding a spin lock in coop mode and transit to preemp mode will cause deadlock on GC _ASSERTE ((m_StateNC & Thread::TSNC_OwnsSpinLock) == 0); -#ifdef ENABLE_CONTRACTS_IMPL - _ASSERTE(!GCForbidden()); - TriggersGC(this); -#endif - // ------------------------------------------------------------------------ // ** WARNING ** WARNING ** WARNING ** WARNING ** WARNING ** WARNING ** | // ------------------------------------------------------------------------ @@ -1414,21 +1409,12 @@ class Thread // ------------------------------------------------------------------------ m_fPreemptiveGCDisabled.StoreWithoutBarrier(0); -#ifdef ENABLE_CONTRACTS - m_ulEnablePreemptiveGCCount ++; -#endif // _DEBUG - - // TODO: VS remove this. no polling on going to preemptive - // Delete RareEnablePreemptiveGC as well. - if (CatchAtSafePointOpportunistic()) - RareEnablePreemptiveGC(); #endif } #if defined(STRESS_HEAP) && defined(_DEBUG) void PerformPreemptiveGC(); #endif - void RareEnablePreemptiveGC(); void PulseGCMode(); //-------------------------------------------------------------- @@ -2804,11 +2790,6 @@ class Thread #endif // TRACK_SYNC -private: -#ifdef ENABLE_CONTRACTS_DATA - ULONG m_ulEnablePreemptiveGCCount; -#endif // _DEBUG - private: // For suspends: CLREvent m_DebugSuspendEvent; diff --git a/src/coreclr/vm/threadsuspend.cpp b/src/coreclr/vm/threadsuspend.cpp index 62cf6bd4deb86..70b89fcdecdf4 100644 --- a/src/coreclr/vm/threadsuspend.cpp +++ b/src/coreclr/vm/threadsuspend.cpp @@ -2108,6 +2108,48 @@ void Thread::RareDisablePreemptiveGC() goto Exit; } +#if defined(STRESS_HEAP) && defined(_DEBUG) + if (!IsDetached()) + { + PerformPreemptiveGC(); + } +#endif + + // TODO: VS can we actually hold TSL here? + if (m_State & TS_DebugSuspendPending) + { + if (!ThreadStore::HoldingThreadStore(this)) + { + #ifdef FEATURE_HIJACK + // TODO: VS can we have hijacks here? why do we remove them? + // Remove any hijacks we might have. + UnhijackThread(); + #endif // FEATURE_HIJACK + + // If GC suspend is in progress we will block while switching to coop mode. + // But if we are doing a non-GC suspend, we need to block now. + // Give the debugger precedence over user suspensions: + while ((m_State & TS_DebugSuspendPending) && !IsInForbidSuspendForDebuggerRegion()) + { + #ifdef DEBUGGING_SUPPORTED + // We don't notify the debugger that this thread is now suspended. We'll just + // let the debugger's helper thread sweep and pick it up. + // We also never take the TSL in here either. + // Life's much simpler this way... + #endif // DEBUGGING_SUPPORTED + + #ifdef LOGGING + { + LOG((LF_CORDB, LL_INFO1000, "[0x%x] SUSPEND: debug suspended while switching to coop mode.\n", GetThreadId())); + } + #endif + // unsets TS_DebugSuspendPending | TS_SyncSuspended + WaitSuspendEvents(); + } + } + } + + // 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 @@ -2186,6 +2228,7 @@ void Thread::RareDisablePreemptiveGC() break; } + // TODO: VS why is this? Have we not just pulsed through GC? __SwitchToThread(0, ++dwSwitchCount); } STRESS_LOG0(LF_SYNC, LL_INFO1000, "RareDisablePreemptiveGC: leaving\n"); @@ -2271,7 +2314,7 @@ void Thread::PreWorkForThreadAbort() #if defined(STRESS_HEAP) && defined(_DEBUG) -// This function is for GC stress testing. Before we enable preemptive GC, let us do a GC +// This function is for GC stress testing. Before we disable preemptive GC, let us do a GC // because GC may happen while the thread is in preemptive GC mode. void Thread::PerformPreemptiveGC() { @@ -2329,76 +2372,6 @@ void Thread::PerformPreemptiveGC() } #endif // STRESS_HEAP && DEBUG -// To leave cooperative mode and enter preemptive mode, if a GC is in progress, we -// no longer care to suspend this thread. But if we are trying to suspend the thread -// for other reasons (e.g. Thread.Suspend()), now is a good time. -// -// Note that it is possible for an N/Direct call to leave the EE without explicitly -// enabling preemptive GC. -void Thread::RareEnablePreemptiveGC() -{ - CONTRACTL { - NOTHROW; - DISABLED(GC_TRIGGERS); // I think this is actually wrong: prevents a p->c->p mode switch inside a NOTRIGGER region. - } - CONTRACTL_END; - - // @todo - Needs a hard SO probe - CONTRACT_VIOLATION(GCViolation|FaultViolation); - - // If we have already received our PROCESS_DETACH during shutdown, there is only one thread in the - // process and no coordination is necessary. - if (IsAtProcessExit()) - return; - - _ASSERTE (!m_fPreemptiveGCDisabled); - - // holding a spin lock in coop mode and transit to preemp mode will cause deadlock on GC - _ASSERTE ((m_StateNC & Thread::TSNC_OwnsSpinLock) == 0); - - _ASSERTE(!MethodDescBackpatchInfoTracker::IsLockOwnedByCurrentThread() || IsInForbidSuspendForDebuggerRegion()); - -#if defined(STRESS_HEAP) && defined(_DEBUG) - if (!IsDetached()) - PerformPreemptiveGC(); -#endif - - STRESS_LOG1(LF_SYNC, LL_INFO100000, "RareEnablePreemptiveGC: entering. Thread state = %x\n", m_State.Load()); - if (!ThreadStore::HoldingThreadStore(this)) - { -#ifdef FEATURE_HIJACK - // Remove any hijacks we might have. - UnhijackThread(); -#endif // FEATURE_HIJACK - - // for GC, the fact that we are leaving the EE means that it no longer needs to - // suspend us. But if we are doing a non-GC suspend, we need to block now. - // Give the debugger precedence over user suspensions: - while ((m_State & TS_DebugSuspendPending) && !IsInForbidSuspendForDebuggerRegion()) - { - -#ifdef DEBUGGING_SUPPORTED - // We don't notify the debugger that this thread is now suspended. We'll just - // let the debugger's helper thread sweep and pick it up. - // We also never take the TSL in here either. - // Life's much simpler this way... - - -#endif // DEBUGGING_SUPPORTED - -#ifdef LOGGING - { - LOG((LF_CORDB, LL_INFO1000, "[0x%x] SUSPEND: suspended while enabling gc.\n", GetThreadId())); - } -#endif - - WaitSuspendEvents(); // sets bits, too - - } - } - STRESS_LOG0(LF_SYNC, LL_INFO100000, "RareEnablePreemptiveGC: leaving.\n"); -} - // Called when we are passing through a safe point in CommonTripThread or // HandleSuspensionForInterruptedThread. Do the right thing with this thread, // which can either mean waiting for the GC to complete, or performing a