From 48a66d5d4838b347c5376757f095dd9d98335abd Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Thu, 6 Mar 2025 17:21:45 -0800 Subject: [PATCH 1/8] Remove helper method frames from Monitors - Convert to the FCALL fast path/QCALL slow path approach - Make EnterHelperResult/LeaveHelperAction into enum class so that they can safely be silently marshaled between native and managed - Move the lockTaken flag handling to managed code so we can share more helpers --- .../src/System/Threading/Monitor.CoreCLR.cs | 185 ++++++++++-- .../classlibnative/bcltype/objectnative.cpp | 110 +++++++ .../classlibnative/bcltype/objectnative.h | 8 + src/coreclr/inc/jithelpers.h | 12 +- src/coreclr/jit/flowgraph.cpp | 6 +- src/coreclr/vm/appdomain.cpp | 4 - src/coreclr/vm/corelib.h | 3 + src/coreclr/vm/ecalllist.h | 8 +- src/coreclr/vm/jithelpers.cpp | 276 ------------------ src/coreclr/vm/jitinterface.h | 59 ---- src/coreclr/vm/metasig.h | 1 + src/coreclr/vm/object.inl | 6 +- src/coreclr/vm/qcallentrypoints.cpp | 3 + src/coreclr/vm/syncblk.cpp | 46 +-- src/coreclr/vm/syncblk.h | 20 +- src/coreclr/vm/syncblk.inl | 70 ++--- 16 files changed, 380 insertions(+), 437 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Threading/Monitor.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Threading/Monitor.CoreCLR.cs index fdfc1c333e9715..c81f1d2ad126a6 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Threading/Monitor.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Threading/Monitor.CoreCLR.cs @@ -30,9 +30,74 @@ public static partial class Monitor ** ** Exceptions: ArgumentNullException if object is null. =========================================================================*/ + public static void Enter(object obj) + { + ArgumentNullException.ThrowIfNull(obj); + + if (!TryEnter_FastPath(obj)) + { + Enter_Slowpath(obj); + } + } + + [MethodImpl(MethodImplOptions.InternalCall)] + private static extern bool TryEnter_FastPath(object obj); + + private enum EnterHelperResult + { + Contention = 0, + Entered = 1, + UseSlowPath = 2 + } + + private enum LeaveHelperAction { + None = 0, + Signal = 1, + Yield = 2, + Contention = 3, + Error = 4, + }; + [MethodImpl(MethodImplOptions.InternalCall)] - public static extern void Enter(object obj); + private static extern EnterHelperResult TryEnter_FastPath_WithTimeout(object obj, int timeout); + + [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "Monitor_Enter_Slowpath")] + private static partial void Enter_Slowpath(ObjectHandleOnStack obj); + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void Enter_Slowpath(object obj) + { + Enter_Slowpath(ObjectHandleOnStack.Create(ref obj)); + } + + [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "Monitor_TryEnter_Slowpath")] + private static partial int TryEnter_Slowpath(ObjectHandleOnStack obj, int timeout); + [MethodImpl(MethodImplOptions.NoInlining)] + private static bool TryEnter_Slowpath(object obj) + { + if (TryEnter_Slowpath(ObjectHandleOnStack.Create(ref obj), 0) != 0) + { + return true; + } + else + { + return false; + } + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static bool TryEnter_Slowpath(object obj, int timeout) + { + if (TryEnter_Slowpath(ObjectHandleOnStack.Create(ref obj), timeout) != 0) + { + return true; + } + else + { + return false; + } + } // Use a ref bool instead of out to ensure that unverifiable code must // initialize this value to something. If we used out, the value @@ -44,7 +109,13 @@ public static void Enter(object obj, ref bool lockTaken) if (lockTaken) ThrowLockTakenException(); - ReliableEnter(obj, ref lockTaken); + ArgumentNullException.ThrowIfNull(obj); + + if (!TryEnter_FastPath(obj)) + { + Enter_Slowpath(obj); + } + lockTaken = true; Debug.Assert(lockTaken); } @@ -55,9 +126,16 @@ private static void ThrowLockTakenException() } [MethodImpl(MethodImplOptions.InternalCall)] - private static extern void ReliableEnter(object obj, ref bool lockTaken); + private static extern LeaveHelperAction Exit_FastPath(object obj); + [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "Monitor_Enter_Slowpath")] + private static partial void Exit_Slowpath(ObjectHandleOnStack obj, LeaveHelperAction exitBehavior); + [MethodImpl(MethodImplOptions.NoInlining)] + private static void Exit_Slowpath(object obj, LeaveHelperAction exitBehavior) + { + Exit_Slowpath(ObjectHandleOnStack.Create(ref obj), exitBehavior); + } /*========================================================================= ** Release the monitor lock. If one or more threads are waiting to acquire the @@ -68,8 +146,35 @@ private static void ThrowLockTakenException() ** SynchronizationLockException if the current thread does not ** own the lock. =========================================================================*/ - [MethodImpl(MethodImplOptions.InternalCall)] - public static extern void Exit(object obj); + public static void Exit(object obj) + { + LeaveHelperAction exitBehavior = Exit_FastPath(obj); + + if (exitBehavior == LeaveHelperAction.None) + return; + + Exit_Slowpath(obj, exitBehavior); + } + + // Used to implement synchronized methods on non Windows-X86 architectures + internal static void ExitIfLockTaken(object obj, ref bool lockTaken) + { + ArgumentNullException.ThrowIfNull(obj); + + if (lockTaken) + { + LeaveHelperAction exitBehavior = Exit_FastPath(obj); + + if (exitBehavior == LeaveHelperAction.None) + { + lockTaken = false; + return; + } + + Exit_Slowpath(obj, exitBehavior); + lockTaken = false; + } + } /*========================================================================= ** Similar to Enter, but will never block. That is, if the current thread can @@ -80,9 +185,41 @@ private static void ThrowLockTakenException() =========================================================================*/ public static bool TryEnter(object obj) { - bool lockTaken = false; - TryEnter(obj, 0, ref lockTaken); - return lockTaken; + ArgumentNullException.ThrowIfNull(obj); + + EnterHelperResult tryEnterResult = TryEnter_FastPath_WithTimeout(obj, 0); + if (tryEnterResult == EnterHelperResult.Entered) + { + return true; + } + else if (tryEnterResult == EnterHelperResult.Contention) + { + return false; + } + + return TryEnter_Slowpath(obj); + } + + private static void TryEnter_Timeout_WithLockTaken(object obj, int timeout, ref bool lockTaken) + { + if (timeout >= -1) + { + EnterHelperResult tryEnterResult = TryEnter_FastPath_WithTimeout(obj, timeout); + if (tryEnterResult == EnterHelperResult.Entered) + { + lockTaken = true; + return; + } + else if (tryEnterResult == EnterHelperResult.Contention) + { + return; + } + } + + if (TryEnter_Slowpath(obj, timeout)) + { + lockTaken = true; + } } // The JIT should inline this method to allow check of lockTaken argument to be optimized out @@ -92,7 +229,9 @@ public static void TryEnter(object obj, ref bool lockTaken) if (lockTaken) ThrowLockTakenException(); - ReliableEnterTimeout(obj, 0, ref lockTaken); + ArgumentNullException.ThrowIfNull(obj); + + TryEnter_WithLockTaken(obj, ref lockTaken); } /*========================================================================= @@ -103,13 +242,24 @@ public static void TryEnter(object obj, ref bool lockTaken) ** Exceptions: ArgumentNullException if object is null. ** ArgumentException if timeout < -1 (Timeout.Infinite). =========================================================================*/ - // The JIT should inline this method to allow check of lockTaken argument to be optimized out - // in the typical case. Note that the method has to be transparent for inlining to be allowed by the VM. public static bool TryEnter(object obj, int millisecondsTimeout) { - bool lockTaken = false; - TryEnter(obj, millisecondsTimeout, ref lockTaken); - return lockTaken; + ArgumentNullException.ThrowIfNull(obj); + + if (millisecondsTimeout >= -1) + { + EnterHelperResult tryEnterResult = TryEnter_FastPath_WithTimeout(obj, millisecondsTimeout); + if (tryEnterResult == EnterHelperResult.Entered) + { + return true; + } + else if (tryEnterResult == EnterHelperResult.Contention) + { + return false; + } + } + + return TryEnter_Slowpath(obj, millisecondsTimeout); } // The JIT should inline this method to allow check of lockTaken argument to be optimized out @@ -119,11 +269,10 @@ public static void TryEnter(object obj, int millisecondsTimeout, ref bool lockTa if (lockTaken) ThrowLockTakenException(); - ReliableEnterTimeout(obj, millisecondsTimeout, ref lockTaken); - } + ArgumentNullException.ThrowIfNull(obj); - [MethodImpl(MethodImplOptions.InternalCall)] - private static extern void ReliableEnterTimeout(object obj, int timeout, ref bool lockTaken); + TryEnter_Timeout_WithLockTaken(obj, millisecondsTimeout, ref lockTaken); + } public static bool IsEntered(object obj) { diff --git a/src/coreclr/classlibnative/bcltype/objectnative.cpp b/src/coreclr/classlibnative/bcltype/objectnative.cpp index 56ed9d3ee84515..7fa4503d2cc43b 100644 --- a/src/coreclr/classlibnative/bcltype/objectnative.cpp +++ b/src/coreclr/classlibnative/bcltype/objectnative.cpp @@ -199,3 +199,113 @@ extern "C" INT64 QCALLTYPE Monitor_GetLockContentionCount() END_QCALL; return result; } + +//======================================================================== +// +// MONITOR HELPERS +// +//======================================================================== + +/*********************************************************************/ +extern "C" void QCALLTYPE Monitor_Enter_Slowpath(QCall::ObjectHandleOnStack objHandle) +{ + QCALL_CONTRACT; + + GCX_COOP(); + + objHandle.Get()->EnterObjMonitor(); +} + +/*********************************************************************/ +#include + +FCIMPL1(FC_BOOL_RET, ObjectNative::Monitor_TryEnter_FastPath, Object* obj) +{ + FCALL_CONTRACT; + + if (obj->TryEnterObjMonitorSpinHelper()) + { + FC_RETURN_BOOL(TRUE); + } + else + { + FC_RETURN_BOOL(FALSE); + } +} +FCIMPLEND + +FCIMPL2(AwareLock::EnterHelperResult, ObjectNative::Monitor_TryEnter_FastPath_WithTimeout, Object* obj, INT32 timeOut) +{ + FCALL_CONTRACT; + + Thread* pCurThread = GetThread(); + + if (pCurThread->CatchAtSafePoint()) + { + return AwareLock::EnterHelperResult::UseSlowPath; + } + + AwareLock::EnterHelperResult result = obj->EnterObjMonitorHelper(pCurThread); + if (result == AwareLock::EnterHelperResult::Contention) + { + if (timeOut == 0) + { + return AwareLock::EnterHelperResult::Contention; + } + + result = obj->EnterObjMonitorHelperSpin(pCurThread); + } + + return result; +} +FCIMPLEND + +#include + +/*********************************************************************/ +extern "C" INT32 QCALLTYPE Monitor_TryEnter_Slowpath(QCall::ObjectHandleOnStack objHandle, INT32 timeOut) +{ + QCALL_CONTRACT; + + GCX_COOP(); + + if (timeOut < -1) + COMPlusThrow(kArgumentOutOfRangeException); + + BOOL result = objHandle.Get()->TryEnterObjMonitor(timeOut); + + return result; +} + +/*********************************************************************/ +extern "C" void QCALLTYPE Monitor_Exit_Slowpath(QCall::ObjectHandleOnStack objHandle, AwareLock::LeaveHelperAction exitBehavior) +{ + QCALL_CONTRACT; + + GCX_COOP(); + + if (exitBehavior != AwareLock::LeaveHelperAction::Signal) + { + if (!objHandle.Get()->LeaveObjMonitor()) + COMPlusThrow(kSynchronizationLockException); + } + else + { + // Signal the event + SyncBlock *psb = objHandle.Get()->PassiveGetSyncBlock(); + if (psb != NULL) + psb->QuickGetMonitor()->Signal(); + } +} + +#include +FCIMPL1(AwareLock::LeaveHelperAction, ObjectNative::Monitor_Exit_FastPath, Object* obj) +{ + FCALL_CONTRACT; + + // Handle the simple case without erecting helper frame + return obj->LeaveObjMonitorHelper(GetThread()); +} +FCIMPLEND +#include + diff --git a/src/coreclr/classlibnative/bcltype/objectnative.h b/src/coreclr/classlibnative/bcltype/objectnative.h index 8178ce79a55270..9dfb597145ae69 100644 --- a/src/coreclr/classlibnative/bcltype/objectnative.h +++ b/src/coreclr/classlibnative/bcltype/objectnative.h @@ -27,6 +27,10 @@ class ObjectNative static FCDECL1(INT32, TryGetHashCode, Object* vThisRef); static FCDECL2(FC_BOOL_RET, ContentEquals, Object *pThisRef, Object *pCompareRef); static FCDECL1(FC_BOOL_RET, IsLockHeld, Object* pThisUNSAFE); + + static FCDECL1(FC_BOOL_RET, Monitor_TryEnter_FastPath, Object* obj); + static FCDECL2(AwareLock::EnterHelperResult, Monitor_TryEnter_FastPath_WithTimeout, Object* obj, INT32 timeout); + static FCDECL1(AwareLock::LeaveHelperAction, Monitor_Exit_FastPath, Object* obj); }; extern "C" INT32 QCALLTYPE ObjectNative_GetHashCodeSlow(QCall::ObjectHandleOnStack objHandle); @@ -35,5 +39,9 @@ extern "C" BOOL QCALLTYPE Monitor_Wait(QCall::ObjectHandleOnStack pThis, INT32 T extern "C" void QCALLTYPE Monitor_Pulse(QCall::ObjectHandleOnStack pThis); extern "C" void QCALLTYPE Monitor_PulseAll(QCall::ObjectHandleOnStack pThis); extern "C" INT64 QCALLTYPE Monitor_GetLockContentionCount(); +extern "C" void QCALLTYPE Monitor_Enter_Slowpath(QCall::ObjectHandleOnStack objHandle); +extern "C" void QCALLTYPE Monitor_Exit_Slowpath(QCall::ObjectHandleOnStack objHandle, AwareLock::LeaveHelperAction exitBehavior); +extern "C" INT32 QCALLTYPE Monitor_TryEnter_Slowpath(QCall::ObjectHandleOnStack objHandle, INT32 timeOut); + #endif // _OBJECTNATIVE_H_ diff --git a/src/coreclr/inc/jithelpers.h b/src/coreclr/inc/jithelpers.h index 38114a9bbfcada..55a8b8fa82bf78 100644 --- a/src/coreclr/inc/jithelpers.h +++ b/src/coreclr/inc/jithelpers.h @@ -141,8 +141,16 @@ JITHELPER(CORINFO_HELP_ENDCATCH, JIT_EndCatch, METHOD__NIL) #endif - JITHELPER(CORINFO_HELP_MON_ENTER, JIT_MonEnterWorker, METHOD__NIL) - JITHELPER(CORINFO_HELP_MON_EXIT, JIT_MonExitWorker, METHOD__NIL) +// +// The legacy x86 monitor helpers do not need a state argument +// +#if defined(FEATURE_EH_FUNCLETS) + DYNAMICJITHELPER(CORINFO_HELP_MON_ENTER, NULL, METHOD__MONITOR__RELIABLEENTER) + DYNAMICJITHELPER(CORINFO_HELP_MON_EXIT, NULL, METHOD__MONITOR__EXIT_IF_TAKEN) +#else + DYNAMICJITHELPER(CORINFO_HELP_MON_ENTER, NULL, METHOD__MONITOR__ENTER) + DYNAMICJITHELPER(CORINFO_HELP_MON_EXIT, NULL, METHOD__MONITOR__EXIT) +#endif JITHELPER(CORINFO_HELP_GETCLASSFROMMETHODPARAM, JIT_GetClassFromMethodParam, METHOD__NIL) DYNAMICJITHELPER(CORINFO_HELP_GETSYNCFROMCLASSHANDLE, NULL, METHOD__RT_TYPE_HANDLE__GETRUNTIMETYPEFROMHANDLE) diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index c900ae003fe1dc..e233d69598e124 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -1365,13 +1365,13 @@ GenTree* Compiler::fgGetCritSectOfStaticMethod() * { * unsigned byte acquired = 0; * try { - * JIT_MonEnterWorker(, &acquired); + * Monitor.Enter(, &acquired); * * *** all the preexisting user code goes here *** * - * JIT_MonExitWorker(, &acquired); + * Monitor.ExitIfTaken(, &acquired); * } fault { - * JIT_MonExitWorker(, &acquired); + * Monitor.ExitIfTaken(, &acquired); * } * L_return: * ret diff --git a/src/coreclr/vm/appdomain.cpp b/src/coreclr/vm/appdomain.cpp index 1ac843760cb065..b91ad353b93bf6 100644 --- a/src/coreclr/vm/appdomain.cpp +++ b/src/coreclr/vm/appdomain.cpp @@ -1139,10 +1139,6 @@ void SystemDomain::LoadBaseSystemClasses() g_pStackFrameIteratorClass = CoreLibBinder::GetClass(CLASS__STACKFRAMEITERATOR); #endif - // Make sure that FCall mapping for Monitor.Enter is initialized. We need it in case Monitor.Enter is used only as JIT helper. - // For more details, see comment in code:JITutil_MonEnterWorker around "__me = GetEEFuncEntryPointMacro(JIT_MonEnter)". - ECall::GetFCallImpl(CoreLibBinder::GetMethod(METHOD__MONITOR__ENTER)); - g_pGetGCStaticBase = CoreLibBinder::GetMethod(METHOD__STATICSHELPERS__GET_GC_STATIC)->GetMultiCallableAddrOfCode(); g_pGetNonGCStaticBase = CoreLibBinder::GetMethod(METHOD__STATICSHELPERS__GET_NONGC_STATIC)->GetMultiCallableAddrOfCode(); g_pPollGC = CoreLibBinder::GetMethod(METHOD__THREAD__POLLGC)->GetMultiCallableAddrOfCode(); diff --git a/src/coreclr/vm/corelib.h b/src/coreclr/vm/corelib.h index e483a08eb1d670..da8964ca52776f 100644 --- a/src/coreclr/vm/corelib.h +++ b/src/coreclr/vm/corelib.h @@ -600,6 +600,9 @@ END_ILLINK_FEATURE_SWITCH() DEFINE_CLASS(MONITOR, Threading, Monitor) DEFINE_METHOD(MONITOR, ENTER, Enter, SM_Obj_RetVoid) +DEFINE_METHOD(MONITOR, EXIT, Exit, SM_Obj_RetVoid) +DEFINE_METHOD(MONITOR, RELIABLEENTER, Enter, SM_Obj_RefBool_RetVoid) +DEFINE_METHOD(MONITOR, EXIT_IF_TAKEN, ExitIfLockTaken, SM_Obj_RefBool_RetVoid) DEFINE_CLASS(THREAD_BLOCKING_INFO, Threading, ThreadBlockingInfo) DEFINE_FIELD(THREAD_BLOCKING_INFO, OFFSET_OF_LOCK_OWNER_OS_THREAD_ID, s_monitorObjectOffsetOfLockOwnerOSThreadId) diff --git a/src/coreclr/vm/ecalllist.h b/src/coreclr/vm/ecalllist.h index 6758a6f007d132..cbf033eabdd487 100644 --- a/src/coreclr/vm/ecalllist.h +++ b/src/coreclr/vm/ecalllist.h @@ -313,11 +313,11 @@ FCFuncStart(gJitInfoFuncs) FCFuncEnd() FCFuncStart(gMonitorFuncs) - FCFuncElement("Enter", JIT_MonEnter) - FCFuncElement("ReliableEnter", JIT_MonReliableEnter) - FCFuncElement("ReliableEnterTimeout", JIT_MonTryEnter) - FCFuncElement("Exit", JIT_MonExit) FCFuncElement("IsEnteredNative", ObjectNative::IsLockHeld) + + FCFuncElement("TryEnter_FastPath", ObjectNative::Monitor_TryEnter_FastPath) + FCFuncElement("TryEnter_FastPath_WithTimeout", ObjectNative::Monitor_TryEnter_FastPath_WithTimeout) + FCFuncElement("Exit_FastPath", ObjectNative::Monitor_Exit_FastPath) FCFuncEnd() FCFuncStart(gRuntimeHelpers) diff --git a/src/coreclr/vm/jithelpers.cpp b/src/coreclr/vm/jithelpers.cpp index 28ac102756360d..0967f4ce833adf 100644 --- a/src/coreclr/vm/jithelpers.cpp +++ b/src/coreclr/vm/jithelpers.cpp @@ -1516,282 +1516,6 @@ HCIMPLEND -//======================================================================== -// -// MONITOR HELPERS -// -//======================================================================== - -/*********************************************************************/ -NOINLINE static void JIT_MonEnter_Helper(Object* obj, BYTE* pbLockTaken, LPVOID __me) -{ - FC_INNER_PROLOG_NO_ME_SETUP(); - - OBJECTREF objRef = ObjectToOBJECTREF(obj); - - // Monitor helpers are used as both hcalls and fcalls, thus we need exact depth. - HELPER_METHOD_FRAME_BEGIN_ATTRIB_1(Frame::FRAME_ATTR_EXACT_DEPTH|Frame::FRAME_ATTR_CAPTURE_DEPTH_2, objRef); - - if (objRef == NULL) - COMPlusThrow(kArgumentNullException); - - GCPROTECT_BEGININTERIOR(pbLockTaken); - - if (GET_THREAD()->CatchAtSafePoint()) - { - GET_THREAD()->PulseGCMode(); - } - objRef->EnterObjMonitor(); - - if (pbLockTaken != 0) *pbLockTaken = 1; - - GCPROTECT_END(); - HELPER_METHOD_FRAME_END(); - - FC_INNER_EPILOG(); -} - -/*********************************************************************/ -#include - -HCIMPL_MONHELPER(JIT_MonEnterWorker_Portable, Object* obj) -{ - FCALL_CONTRACT; - - if (obj != nullptr && obj->TryEnterObjMonitorSpinHelper()) - { - MONHELPER_STATE(*pbLockTaken = 1); - return; - } - - FC_INNER_RETURN_VOID(JIT_MonEnter_Helper(obj, MONHELPER_ARG, GetEEFuncEntryPointMacro(JIT_MonEnter))); -} -HCIMPLEND - -HCIMPL1(void, JIT_MonEnter_Portable, Object* obj) -{ - FCALL_CONTRACT; - - if (obj != nullptr && obj->TryEnterObjMonitorSpinHelper()) - { - return; - } - - FC_INNER_RETURN_VOID(JIT_MonEnter_Helper(obj, NULL, GetEEFuncEntryPointMacro(JIT_MonEnter))); -} -HCIMPLEND - -HCIMPL2(void, JIT_MonReliableEnter_Portable, Object* obj, BYTE* pbLockTaken) -{ - FCALL_CONTRACT; - - if (obj != nullptr && obj->TryEnterObjMonitorSpinHelper()) - { - *pbLockTaken = 1; - return; - } - - FC_INNER_RETURN_VOID(JIT_MonEnter_Helper(obj, pbLockTaken, GetEEFuncEntryPointMacro(JIT_MonReliableEnter))); -} -HCIMPLEND - -#include - - -/*********************************************************************/ -NOINLINE static void JIT_MonTryEnter_Helper(Object* obj, INT32 timeOut, BYTE* pbLockTaken) -{ - FC_INNER_PROLOG(JIT_MonTryEnter); - - OBJECTREF objRef = ObjectToOBJECTREF(obj); - - // Monitor helpers are used as both hcalls and fcalls, thus we need exact depth. - HELPER_METHOD_FRAME_BEGIN_ATTRIB_1(Frame::FRAME_ATTR_EXACT_DEPTH|Frame::FRAME_ATTR_CAPTURE_DEPTH_2, objRef); - - if (objRef == NULL) - COMPlusThrow(kArgumentNullException); - - if (timeOut < -1) - COMPlusThrow(kArgumentOutOfRangeException); - - GCPROTECT_BEGININTERIOR(pbLockTaken); - - if (GET_THREAD()->CatchAtSafePoint()) - { - GET_THREAD()->PulseGCMode(); - } - - BOOL result = objRef->TryEnterObjMonitor(timeOut); - *pbLockTaken = result != FALSE; - - GCPROTECT_END(); - HELPER_METHOD_FRAME_END(); - - FC_INNER_EPILOG(); -} - -#include -HCIMPL3(void, JIT_MonTryEnter_Portable, Object* obj, INT32 timeOut, BYTE* pbLockTaken) -{ - FCALL_CONTRACT; - - AwareLock::EnterHelperResult result; - Thread * pCurThread; - - if (obj == NULL) - { - goto FramedLockHelper; - } - - if (timeOut < -1) - { - goto FramedLockHelper; - } - - pCurThread = GetThread(); - - if (pCurThread->CatchAtSafePoint()) - { - goto FramedLockHelper; - } - - result = obj->EnterObjMonitorHelper(pCurThread); - if (result == AwareLock::EnterHelperResult_Entered) - { - *pbLockTaken = 1; - return; - } - if (result == AwareLock::EnterHelperResult_Contention) - { - if (timeOut == 0) - { - return; - } - - result = obj->EnterObjMonitorHelperSpin(pCurThread); - if (result == AwareLock::EnterHelperResult_Entered) - { - *pbLockTaken = 1; - return; - } - } - -FramedLockHelper: - FC_INNER_RETURN_VOID(JIT_MonTryEnter_Helper(obj, timeOut, pbLockTaken)); -} -HCIMPLEND -#include - -/*********************************************************************/ -NOINLINE static void JIT_MonExit_Helper(Object* obj, BYTE* pbLockTaken) -{ - FC_INNER_PROLOG(JIT_MonExit); - - OBJECTREF objRef = ObjectToOBJECTREF(obj); - - // Monitor helpers are used as both hcalls and fcalls, thus we need exact depth. - HELPER_METHOD_FRAME_BEGIN_ATTRIB_1(Frame::FRAME_ATTR_NO_THREAD_ABORT|Frame::FRAME_ATTR_EXACT_DEPTH|Frame::FRAME_ATTR_CAPTURE_DEPTH_2, objRef); - - if (objRef == NULL) - COMPlusThrow(kArgumentNullException); - - if (!objRef->LeaveObjMonitor()) - COMPlusThrow(kSynchronizationLockException); - - if (pbLockTaken != 0) *pbLockTaken = 0; - - if (GET_THREAD()->IsAbortRequested()) { - GET_THREAD()->HandleThreadAbort(); - } - - HELPER_METHOD_FRAME_END(); - - FC_INNER_EPILOG(); -} - -NOINLINE static void JIT_MonExit_Signal(Object* obj) -{ - FC_INNER_PROLOG(JIT_MonExit); - - OBJECTREF objRef = ObjectToOBJECTREF(obj); - - // Monitor helpers are used as both hcalls and fcalls, thus we need exact depth. - HELPER_METHOD_FRAME_BEGIN_ATTRIB_1(Frame::FRAME_ATTR_NO_THREAD_ABORT|Frame::FRAME_ATTR_EXACT_DEPTH|Frame::FRAME_ATTR_CAPTURE_DEPTH_2, objRef); - - // Signal the event - SyncBlock *psb = objRef->PassiveGetSyncBlock(); - if (psb != NULL) - psb->QuickGetMonitor()->Signal(); - - if (GET_THREAD()->IsAbortRequested()) { - GET_THREAD()->HandleThreadAbort(); - } - - HELPER_METHOD_FRAME_END(); - - FC_INNER_EPILOG(); -} - -#include -FCIMPL1(void, JIT_MonExit_Portable, Object* obj) -{ - FCALL_CONTRACT; - - AwareLock::LeaveHelperAction action; - - if (obj == NULL) - { - goto FramedLockHelper; - } - - // Handle the simple case without erecting helper frame - action = obj->LeaveObjMonitorHelper(GetThread()); - if (action == AwareLock::LeaveHelperAction_None) - { - return; - } - if (action == AwareLock::LeaveHelperAction_Signal) - { - FC_INNER_RETURN_VOID(JIT_MonExit_Signal(obj)); - } - -FramedLockHelper: - FC_INNER_RETURN_VOID(JIT_MonExit_Helper(obj, NULL)); -} -HCIMPLEND - -HCIMPL_MONHELPER(JIT_MonExitWorker_Portable, Object* obj) -{ - FCALL_CONTRACT; - - MONHELPER_STATE(_ASSERTE(pbLockTaken != NULL)); - MONHELPER_STATE(if (*pbLockTaken == 0) return;) - - AwareLock::LeaveHelperAction action; - - if (obj == NULL) - { - goto FramedLockHelper; - } - - // Handle the simple case without erecting helper frame - action = obj->LeaveObjMonitorHelper(GetThread()); - if (action == AwareLock::LeaveHelperAction_None) - { - MONHELPER_STATE(*pbLockTaken = 0;) - return; - } - if (action == AwareLock::LeaveHelperAction_Signal) - { - MONHELPER_STATE(*pbLockTaken = 0;) - FC_INNER_RETURN_VOID(JIT_MonExit_Signal(obj)); - } - -FramedLockHelper: - FC_INNER_RETURN_VOID(JIT_MonExit_Helper(obj, MONHELPER_ARG)); -} -HCIMPLEND -#include //======================================================================== // diff --git a/src/coreclr/vm/jitinterface.h b/src/coreclr/vm/jitinterface.h index 7f1835e458a53a..a38e4ee3549de0 100644 --- a/src/coreclr/vm/jitinterface.h +++ b/src/coreclr/vm/jitinterface.h @@ -84,25 +84,6 @@ BOOL LoadDynamicInfoEntry(Module *currentModule, SIZE_T *entry, BOOL mayUsePrecompiledNDirectMethods = TRUE); -// -// The legacy x86 monitor helpers do not need a state argument -// -#if !defined(TARGET_X86) - -#define FCDECL_MONHELPER(funcname, arg) FCDECL2(void, funcname, arg, BYTE* pbLockTaken) -#define HCIMPL_MONHELPER(funcname, arg) HCIMPL2(void, funcname, arg, BYTE* pbLockTaken) -#define MONHELPER_STATE(x) x -#define MONHELPER_ARG pbLockTaken - -#else - -#define FCDECL_MONHELPER(funcname, arg) FCDECL1(void, funcname, arg) -#define HCIMPL_MONHELPER(funcname, arg) HCIMPL1(void, funcname, arg) -#define MONHELPER_STATE(x) -#define MONHELPER_ARG NULL - -#endif // TARGET_X86 - // These must be implemented in assembly and generate a TransitionBlock then calling JIT_PatchpointWorkerWithPolicy in order to actually be used. EXTERN_C FCDECL2(void, JIT_Patchpoint, int* counter, int ilOffset); EXTERN_C FCDECL1(void, JIT_PartialCompilationPatchpoint, int ilOffset); @@ -115,42 +96,6 @@ EXTERN_C FCDECL1(void, JIT_PartialCompilationPatchpoint, int ilOffset); EXTERN_C FCDECL0(void, JIT_PollGC); -#ifndef JIT_MonEnter -#define JIT_MonEnter JIT_MonEnter_Portable -#endif -EXTERN_C FCDECL1(void, JIT_MonEnter, Object *obj); -EXTERN_C FCDECL1(void, JIT_MonEnter_Portable, Object *obj); - -#ifndef JIT_MonEnterWorker -#define JIT_MonEnterWorker JIT_MonEnterWorker_Portable -#endif -EXTERN_C FCDECL_MONHELPER(JIT_MonEnterWorker, Object *obj); -EXTERN_C FCDECL_MONHELPER(JIT_MonEnterWorker_Portable, Object *obj); - -#ifndef JIT_MonReliableEnter -#define JIT_MonReliableEnter JIT_MonReliableEnter_Portable -#endif -EXTERN_C FCDECL2(void, JIT_MonReliableEnter, Object* obj, BYTE *tookLock); -EXTERN_C FCDECL2(void, JIT_MonReliableEnter_Portable, Object* obj, BYTE *tookLock); - -#ifndef JIT_MonTryEnter -#define JIT_MonTryEnter JIT_MonTryEnter_Portable -#endif -EXTERN_C FCDECL3(void, JIT_MonTryEnter, Object *obj, INT32 timeout, BYTE* pbLockTaken); -EXTERN_C FCDECL3(void, JIT_MonTryEnter_Portable, Object *obj, INT32 timeout, BYTE* pbLockTaken); - -#ifndef JIT_MonExit -#define JIT_MonExit JIT_MonExit_Portable -#endif -EXTERN_C FCDECL1(void, JIT_MonExit, Object *obj); -EXTERN_C FCDECL1(void, JIT_MonExit_Portable, Object *obj); - -#ifndef JIT_MonExitWorker -#define JIT_MonExitWorker JIT_MonExitWorker_Portable -#endif -EXTERN_C FCDECL_MONHELPER(JIT_MonExitWorker, Object *obj); -EXTERN_C FCDECL_MONHELPER(JIT_MonExitWorker_Portable, Object *obj); - #ifndef JIT_GetGCStaticBase #define JIT_GetGCStaticBase NULL #else @@ -209,12 +154,8 @@ extern FCDECL2(Object*, JIT_NewArr1VC_MP_FastPortable, CORINFO_CLASS_HANDLE arra extern FCDECL2(Object*, JIT_NewArr1OBJ_MP_FastPortable, CORINFO_CLASS_HANDLE arrayMT, INT_PTR size); extern FCDECL2(Object*, JIT_NewArr1, CORINFO_CLASS_HANDLE arrayMT, INT_PTR size); -EXTERN_C FCDECL_MONHELPER(JITutil_MonEnterWorker, Object* obj); EXTERN_C FCDECL2(void, JITutil_MonReliableEnter, Object* obj, BYTE* pbLockTaken); EXTERN_C FCDECL3(void, JITutil_MonTryEnter, Object* obj, INT32 timeOut, BYTE* pbLockTaken); -EXTERN_C FCDECL_MONHELPER(JITutil_MonExitWorker, Object* obj); -EXTERN_C FCDECL_MONHELPER(JITutil_MonSignal, AwareLock* lock); -EXTERN_C FCDECL_MONHELPER(JITutil_MonContention, AwareLock* awarelock); EXTERN_C FCDECL2(void, JITutil_MonReliableContention, AwareLock* awarelock, BYTE* pbLockTaken); EXTERN_C FCDECL1(void*, JIT_GetNonGCStaticBase_Helper, MethodTable *pMT); diff --git a/src/coreclr/vm/metasig.h b/src/coreclr/vm/metasig.h index 3d279240f0cbb3..4da1c332bc676b 100644 --- a/src/coreclr/vm/metasig.h +++ b/src/coreclr/vm/metasig.h @@ -179,6 +179,7 @@ DEFINE_METASIG(SM(VoidPtr_RetVoidPtr, P(v), P(v))) DEFINE_METASIG(SM(Obj_VoidPtr_RetVoidPtr, j P(v), P(v))) DEFINE_METASIG(SM(Obj_IntPtr_RetObj, j I, j)) DEFINE_METASIG(SM(Obj_RefIntPtr_RetVoid, j r(I), v)) +DEFINE_METASIG(SM(Obj_RefBool_RetVoid, j r(F), v)) DEFINE_METASIG(SM(Obj_IntPtr_RetVoid, j I, v)) DEFINE_METASIG(SM(Obj_IntPtr_RetBool, j I, F)) DEFINE_METASIG(SM(Obj_IntPtr_IntPtr_Int_RetIntPtr, j I I i, I)) diff --git a/src/coreclr/vm/object.inl b/src/coreclr/vm/object.inl index 92f26ec6d094f1..9b11f554dcf8a6 100644 --- a/src/coreclr/vm/object.inl +++ b/src/coreclr/vm/object.inl @@ -120,14 +120,14 @@ FORCEINLINE bool Object::TryEnterObjMonitorSpinHelper() } AwareLock::EnterHelperResult result = EnterObjMonitorHelper(pCurThread); - if (result == AwareLock::EnterHelperResult_Entered) + if (result == AwareLock::EnterHelperResult::Entered) { return true; } - if (result == AwareLock::EnterHelperResult_Contention) + if (result == AwareLock::EnterHelperResult::Contention) { result = EnterObjMonitorHelperSpin(pCurThread); - if (result == AwareLock::EnterHelperResult_Entered) + if (result == AwareLock::EnterHelperResult::Entered) { return true; } diff --git a/src/coreclr/vm/qcallentrypoints.cpp b/src/coreclr/vm/qcallentrypoints.cpp index 085468f030e9b1..aebd65e887cfa7 100644 --- a/src/coreclr/vm/qcallentrypoints.cpp +++ b/src/coreclr/vm/qcallentrypoints.cpp @@ -401,6 +401,9 @@ static const Entry s_QCall[] = DllImportEntry(Monitor_Pulse) DllImportEntry(Monitor_PulseAll) DllImportEntry(Monitor_GetLockContentionCount) + DllImportEntry(Monitor_Enter_Slowpath) + DllImportEntry(Monitor_TryEnter_Slowpath) + DllImportEntry(Monitor_Exit_Slowpath) DllImportEntry(MetadataImport_Enum) DllImportEntry(ReflectionInvocation_RunClassConstructor) DllImportEntry(ReflectionInvocation_RunModuleConstructor) diff --git a/src/coreclr/vm/syncblk.cpp b/src/coreclr/vm/syncblk.cpp index deff999bb6a27b..073dc9c3f3f839 100644 --- a/src/coreclr/vm/syncblk.cpp +++ b/src/coreclr/vm/syncblk.cpp @@ -1564,7 +1564,7 @@ AwareLock::EnterHelperResult ObjHeader::EnterObjMonitorHelperSpin(Thread* pCurTh if (g_SystemInfo.dwNumberOfProcessors == 1) { - return AwareLock::EnterHelperResult_Contention; + return AwareLock::EnterHelperResult::Contention; } YieldProcessorNormalizationInfo normalizationInfo; @@ -1582,7 +1582,7 @@ AwareLock::EnterHelperResult ObjHeader::EnterObjMonitorHelperSpin(Thread* pCurTh // If we have a hash code already, we need to create a sync block if (oldValue & BIT_SBLK_IS_HASHCODE) { - return AwareLock::EnterHelperResult_UseSlowPath; + return AwareLock::EnterHelperResult::UseSlowPath; } SyncBlock *syncBlock = g_pSyncTable[oldValue & MASK_SYNCBLOCKINDEX].m_SyncBlock; @@ -1590,7 +1590,7 @@ AwareLock::EnterHelperResult ObjHeader::EnterObjMonitorHelperSpin(Thread* pCurTh AwareLock *awareLock = &syncBlock->m_Monitor; AwareLock::EnterHelperResult result = awareLock->TryEnterBeforeSpinLoopHelper(pCurThread); - if (result != AwareLock::EnterHelperResult_Contention) + if (result != AwareLock::EnterHelperResult::Contention) { return result; } @@ -1610,11 +1610,11 @@ AwareLock::EnterHelperResult ObjHeader::EnterObjMonitorHelperSpin(Thread* pCurTh } result = awareLock->TryEnterInsideSpinLoopHelper(pCurThread); - if (result == AwareLock::EnterHelperResult_Entered) + if (result == AwareLock::EnterHelperResult::Entered) { - return AwareLock::EnterHelperResult_Entered; + return AwareLock::EnterHelperResult::Entered; } - if (result == AwareLock::EnterHelperResult_UseSlowPath) + if (result == AwareLock::EnterHelperResult::UseSlowPath) { break; } @@ -1623,7 +1623,7 @@ AwareLock::EnterHelperResult ObjHeader::EnterObjMonitorHelperSpin(Thread* pCurTh if (awareLock->TryEnterAfterSpinLoopHelper(pCurThread)) { - return AwareLock::EnterHelperResult_Entered; + return AwareLock::EnterHelperResult::Entered; } break; } @@ -1635,7 +1635,7 @@ AwareLock::EnterHelperResult ObjHeader::EnterObjMonitorHelperSpin(Thread* pCurTh { if (tid > SBLK_MASK_LOCK_THREADID) { - return AwareLock::EnterHelperResult_UseSlowPath; + return AwareLock::EnterHelperResult::UseSlowPath; } LONG newValue = oldValue | tid; @@ -1645,7 +1645,7 @@ AwareLock::EnterHelperResult ObjHeader::EnterObjMonitorHelperSpin(Thread* pCurTh if (InterlockedCompareExchangeAcquire((LONG*)&m_SyncBlockValue, newValue, oldValue) == oldValue) #endif { - return AwareLock::EnterHelperResult_Entered; + return AwareLock::EnterHelperResult::Entered; } continue; @@ -1663,7 +1663,7 @@ AwareLock::EnterHelperResult ObjHeader::EnterObjMonitorHelperSpin(Thread* pCurTh tid != (DWORD)(oldValue & SBLK_MASK_LOCK_THREADID)); } - return AwareLock::EnterHelperResult_Contention; + return AwareLock::EnterHelperResult::Contention; } BOOL ObjHeader::LeaveObjMonitor() @@ -1687,10 +1687,10 @@ BOOL ObjHeader::LeaveObjMonitor() switch(action) { - case AwareLock::LeaveHelperAction_None: + case AwareLock::LeaveHelperAction::None: // We are done return TRUE; - case AwareLock::LeaveHelperAction_Signal: + case AwareLock::LeaveHelperAction::Signal: { // Signal the event SyncBlock *psb = thisObj->GetHeader ()->PassiveGetSyncBlock(); @@ -1698,10 +1698,10 @@ BOOL ObjHeader::LeaveObjMonitor() psb->QuickGetMonitor()->Signal(); } return TRUE; - case AwareLock::LeaveHelperAction_Yield: + case AwareLock::LeaveHelperAction::Yield: YieldProcessorNormalized(); continue; - case AwareLock::LeaveHelperAction_Contention: + case AwareLock::LeaveHelperAction::Contention: // Some thread is updating the syncblock value. { //protect the object before switching mode @@ -1713,7 +1713,7 @@ BOOL ObjHeader::LeaveObjMonitor() continue; default: // Must be an error otherwise - ignore it - _ASSERTE(action == AwareLock::LeaveHelperAction_Error); + _ASSERTE(action == AwareLock::LeaveHelperAction::Error); return FALSE; } } @@ -1739,10 +1739,10 @@ BOOL ObjHeader::LeaveObjMonitorAtException() switch(action) { - case AwareLock::LeaveHelperAction_None: + case AwareLock::LeaveHelperAction::None: // We are done return TRUE; - case AwareLock::LeaveHelperAction_Signal: + case AwareLock::LeaveHelperAction::Signal: { // Signal the event SyncBlock *psb = PassiveGetSyncBlock(); @@ -1750,10 +1750,10 @@ BOOL ObjHeader::LeaveObjMonitorAtException() psb->QuickGetMonitor()->Signal(); } return TRUE; - case AwareLock::LeaveHelperAction_Yield: + case AwareLock::LeaveHelperAction::Yield: YieldProcessorNormalized(); continue; - case AwareLock::LeaveHelperAction_Contention: + case AwareLock::LeaveHelperAction::Contention: // Some thread is updating the syncblock value. // // We never toggle GC mode while holding the spinlock (BeginNoTriggerGC/EndNoTriggerGC @@ -1766,7 +1766,7 @@ BOOL ObjHeader::LeaveObjMonitorAtException() continue; default: // Must be an error otherwise - ignore it - _ASSERTE(action == AwareLock::LeaveHelperAction_Error); + _ASSERTE(action == AwareLock::LeaveHelperAction::Error); return FALSE; } } @@ -2755,16 +2755,16 @@ BOOL AwareLock::Leave() switch(action) { - case AwareLock::LeaveHelperAction_None: + case AwareLock::LeaveHelperAction::None: // We are done return TRUE; - case AwareLock::LeaveHelperAction_Signal: + case AwareLock::LeaveHelperAction::Signal: // Signal the event Signal(); return TRUE; default: // Must be an error otherwise - _ASSERTE(action == AwareLock::LeaveHelperAction_Error); + _ASSERTE(action == AwareLock::LeaveHelperAction::Error); return FALSE; } } diff --git a/src/coreclr/vm/syncblk.h b/src/coreclr/vm/syncblk.h index 705f1d386f2bf2..b9498ec1549999 100644 --- a/src/coreclr/vm/syncblk.h +++ b/src/coreclr/vm/syncblk.h @@ -162,18 +162,18 @@ class AwareLock friend class SyncBlock; public: - enum EnterHelperResult { - EnterHelperResult_Entered, - EnterHelperResult_Contention, - EnterHelperResult_UseSlowPath + enum class EnterHelperResult : INT32 { + Contention, + Entered, + UseSlowPath }; - enum LeaveHelperAction { - LeaveHelperAction_None, - LeaveHelperAction_Signal, - LeaveHelperAction_Yield, - LeaveHelperAction_Contention, - LeaveHelperAction_Error, + enum class LeaveHelperAction : INT32 { + None, + Signal, + Yield, + Contention, + Error, }; private: diff --git a/src/coreclr/vm/syncblk.inl b/src/coreclr/vm/syncblk.inl index 6042c6e08f9fa5..be935d084840b5 100644 --- a/src/coreclr/vm/syncblk.inl +++ b/src/coreclr/vm/syncblk.inl @@ -159,13 +159,13 @@ FORCEINLINE AwareLock::EnterHelperResult AwareLock::LockState::InterlockedTry_Lo } else if (state.ShouldNotPreemptWaiters() || !newState.TryIncrementSpinnerCount()) { - return EnterHelperResult_UseSlowPath; + return EnterHelperResult::UseSlowPath; } LockState stateBeforeUpdate = CompareExchange(newState, state); if (stateBeforeUpdate == state) { - return state.ShouldNonWaiterAttemptToAcquireLock() ? EnterHelperResult_Entered : EnterHelperResult_Contention; + return state.ShouldNonWaiterAttemptToAcquireLock() ? EnterHelperResult::Entered : EnterHelperResult::Contention; } state = stateBeforeUpdate; @@ -183,7 +183,7 @@ FORCEINLINE AwareLock::EnterHelperResult AwareLock::LockState::InterlockedTry_Lo _ASSERTE(state.HasAnySpinners()); if (!state.ShouldNonWaiterAttemptToAcquireLock()) { - return state.ShouldNotPreemptWaiters() ? EnterHelperResult_UseSlowPath : EnterHelperResult_Contention; + return state.ShouldNotPreemptWaiters() ? EnterHelperResult::UseSlowPath : EnterHelperResult::Contention; } LockState newState = state; @@ -193,7 +193,7 @@ FORCEINLINE AwareLock::EnterHelperResult AwareLock::LockState::InterlockedTry_Lo LockState stateBeforeUpdate = CompareExchange(newState, state); if (stateBeforeUpdate == state) { - return EnterHelperResult_Entered; + return EnterHelperResult::Entered; } state = stateBeforeUpdate; @@ -509,16 +509,16 @@ FORCEINLINE AwareLock::EnterHelperResult AwareLock::TryEnterBeforeSpinLoopHelper if (m_lockState.InterlockedTrySetShouldNotPreemptWaitersIfNecessary(this, state)) { // This thread currently should not preempt waiters, just wait - return EnterHelperResult_UseSlowPath; + return EnterHelperResult::UseSlowPath; } // Not a recursive enter, try to acquire the lock or register the spinner EnterHelperResult result = m_lockState.InterlockedTry_LockOrRegisterSpinner(state); - if (result != EnterHelperResult_Entered) + if (result != EnterHelperResult::Entered) { - // EnterHelperResult_Contention: + // EnterHelperResult::Contention: // Lock was not acquired and the spinner was registered - // EnterHelperResult_UseSlowPath: + // EnterHelperResult::UseSlowPath: // This thread currently should not preempt waiters, or we reached the maximum number of spinners, just wait return result; } @@ -527,12 +527,12 @@ FORCEINLINE AwareLock::EnterHelperResult AwareLock::TryEnterBeforeSpinLoopHelper m_HoldingThreadId = pCurThread->GetThreadId(); m_HoldingOSThreadId = pCurThread->GetOSThreadId64(); m_Recursion = 1; - return EnterHelperResult_Entered; + return EnterHelperResult::Entered; } // Recursive enter m_Recursion++; - return EnterHelperResult_Entered; + return EnterHelperResult::Entered; } FORCEINLINE AwareLock::EnterHelperResult AwareLock::TryEnterInsideSpinLoopHelper(Thread *pCurThread) @@ -546,11 +546,11 @@ FORCEINLINE AwareLock::EnterHelperResult AwareLock::TryEnterInsideSpinLoopHelper // Try to acquire the lock and unregister the spinner. The recursive case is not checked here because // TryEnterBeforeSpinLoopHelper() would have taken care of that case before the spin loop. EnterHelperResult result = m_lockState.InterlockedTry_LockAndUnregisterSpinner(); - if (result != EnterHelperResult_Entered) + if (result != EnterHelperResult::Entered) { - // EnterHelperResult_Contention: + // EnterHelperResult::Contention: // Lock was not acquired and the spinner was not unregistered - // EnterHelperResult_UseSlowPath: + // EnterHelperResult::UseSlowPath: // This thread currently should not preempt waiters, stop spinning and just wait return result; } @@ -559,7 +559,7 @@ FORCEINLINE AwareLock::EnterHelperResult AwareLock::TryEnterInsideSpinLoopHelper m_HoldingThreadId = pCurThread->GetThreadId(); m_HoldingOSThreadId = pCurThread->GetOSThreadId64(); m_Recursion = 1; - return EnterHelperResult_Entered; + return EnterHelperResult::Entered; } FORCEINLINE bool AwareLock::TryEnterAfterSpinLoopHelper(Thread *pCurThread) @@ -603,7 +603,7 @@ FORCEINLINE AwareLock::EnterHelperResult ObjHeader::EnterObjMonitorHelper(Thread DWORD tid = pCurThread->GetThreadId(); if (tid > SBLK_MASK_LOCK_THREADID) { - return AwareLock::EnterHelperResult_UseSlowPath; + return AwareLock::EnterHelperResult::UseSlowPath; } LONG newValue = oldValue | tid; @@ -613,10 +613,10 @@ FORCEINLINE AwareLock::EnterHelperResult ObjHeader::EnterObjMonitorHelper(Thread if (InterlockedCompareExchangeAcquire((LONG*)&m_SyncBlockValue, newValue, oldValue) == oldValue) #endif { - return AwareLock::EnterHelperResult_Entered; + return AwareLock::EnterHelperResult::Entered; } - return AwareLock::EnterHelperResult_Contention; + return AwareLock::EnterHelperResult::Contention; } if (oldValue & BIT_SBLK_IS_HASH_OR_SYNCBLKINDEX) @@ -624,30 +624,30 @@ FORCEINLINE AwareLock::EnterHelperResult ObjHeader::EnterObjMonitorHelper(Thread // If we have a hash code already, we need to create a sync block if (oldValue & BIT_SBLK_IS_HASHCODE) { - return AwareLock::EnterHelperResult_UseSlowPath; + return AwareLock::EnterHelperResult::UseSlowPath; } SyncBlock *syncBlock = g_pSyncTable[oldValue & MASK_SYNCBLOCKINDEX].m_SyncBlock; _ASSERTE(syncBlock != NULL); if (syncBlock->m_Monitor.TryEnterHelper(pCurThread)) { - return AwareLock::EnterHelperResult_Entered; + return AwareLock::EnterHelperResult::Entered; } - return AwareLock::EnterHelperResult_Contention; + return AwareLock::EnterHelperResult::Contention; } // The header is transitioning - use the slow path if (oldValue & BIT_SBLK_SPIN_LOCK) { - return AwareLock::EnterHelperResult_UseSlowPath; + return AwareLock::EnterHelperResult::UseSlowPath; } // Here we know we have the "thin lock" layout, but the lock is not free. // It could still be the recursion case - compare the thread id to check if (pCurThread->GetThreadId() != (DWORD)(oldValue & SBLK_MASK_LOCK_THREADID)) { - return AwareLock::EnterHelperResult_Contention; + return AwareLock::EnterHelperResult::Contention; } // Ok, the thread id matches, it's the recursion case. @@ -656,7 +656,7 @@ FORCEINLINE AwareLock::EnterHelperResult ObjHeader::EnterObjMonitorHelper(Thread if ((newValue & SBLK_MASK_LOCK_RECLEVEL) == 0) { - return AwareLock::EnterHelperResult_UseSlowPath; + return AwareLock::EnterHelperResult::UseSlowPath; } #if defined(TARGET_WINDOWS) && defined(TARGET_ARM64) @@ -665,12 +665,12 @@ FORCEINLINE AwareLock::EnterHelperResult ObjHeader::EnterObjMonitorHelper(Thread if (InterlockedCompareExchangeAcquire((LONG*)&m_SyncBlockValue, newValue, oldValue) == oldValue) #endif { - return AwareLock::EnterHelperResult_Entered; + return AwareLock::EnterHelperResult::Entered; } // Use the slow path instead of spinning. The compare-exchange above would not fail often, and it's not worth forcing the // spin loop that typically follows the call to this function to check the recursive case, so just bail to the slow path. - return AwareLock::EnterHelperResult_UseSlowPath; + return AwareLock::EnterHelperResult::UseSlowPath; } // Helper encapsulating the core logic for releasing monitor. Returns what kind of @@ -684,7 +684,7 @@ FORCEINLINE AwareLock::LeaveHelperAction AwareLock::LeaveHelper(Thread* pCurThre } CONTRACTL_END; if (m_HoldingThreadId != pCurThread->GetThreadId()) - return AwareLock::LeaveHelperAction_Error; + return AwareLock::LeaveHelperAction::Error; _ASSERTE(m_lockState.VolatileLoadWithoutBarrier().IsLocked()); _ASSERTE(m_Recursion >= 1); @@ -704,13 +704,13 @@ FORCEINLINE AwareLock::LeaveHelperAction AwareLock::LeaveHelper(Thread* pCurThre // Clear lock bit and determine whether we must signal a waiter to wake if (!m_lockState.InterlockedUnlock()) { - return AwareLock::LeaveHelperAction_None; + return AwareLock::LeaveHelperAction::None; } // There is a waiter and we must signal a waiter to wake - return AwareLock::LeaveHelperAction_Signal; + return AwareLock::LeaveHelperAction::Signal; } - return AwareLock::LeaveHelperAction_None; + return AwareLock::LeaveHelperAction::None; } // Helper encapsulating the core logic for releasing monitor. Returns what kind of @@ -730,7 +730,7 @@ FORCEINLINE AwareLock::LeaveHelperAction ObjHeader::LeaveObjMonitorHelper(Thread if ((syncBlockValue & SBLK_MASK_LOCK_THREADID) != pCurThread->GetThreadId()) { // This thread does not own the lock. - return AwareLock::LeaveHelperAction_Error; + return AwareLock::LeaveHelperAction::Error; } if (!(syncBlockValue & SBLK_MASK_LOCK_RECLEVEL)) @@ -744,7 +744,7 @@ FORCEINLINE AwareLock::LeaveHelperAction ObjHeader::LeaveObjMonitorHelper(Thread if (InterlockedCompareExchangeRelease((LONG*)&m_SyncBlockValue, newValue, syncBlockValue) != (LONG)syncBlockValue) #endif { - return AwareLock::LeaveHelperAction_Yield; + return AwareLock::LeaveHelperAction::Yield; } } else @@ -757,11 +757,11 @@ FORCEINLINE AwareLock::LeaveHelperAction ObjHeader::LeaveObjMonitorHelper(Thread if (InterlockedCompareExchangeRelease((LONG*)&m_SyncBlockValue, newValue, syncBlockValue) != (LONG)syncBlockValue) #endif { - return AwareLock::LeaveHelperAction_Yield; + return AwareLock::LeaveHelperAction::Yield; } } - return AwareLock::LeaveHelperAction_None; + return AwareLock::LeaveHelperAction::None; } if ((syncBlockValue & (BIT_SBLK_SPIN_LOCK + BIT_SBLK_IS_HASHCODE)) == 0) @@ -774,11 +774,11 @@ FORCEINLINE AwareLock::LeaveHelperAction ObjHeader::LeaveObjMonitorHelper(Thread if (syncBlockValue & BIT_SBLK_SPIN_LOCK) { - return AwareLock::LeaveHelperAction_Contention; + return AwareLock::LeaveHelperAction::Contention; } // This thread does not own the lock. - return AwareLock::LeaveHelperAction_Error; + return AwareLock::LeaveHelperAction::Error; } #endif // DACCESS_COMPILE From 5ba5cf751037d9b96534d4ee0e94b3ec60696ffe Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Fri, 7 Mar 2025 09:25:09 -0800 Subject: [PATCH 2/8] Refactoring oops --- .../src/System/Threading/Monitor.CoreCLR.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Threading/Monitor.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Threading/Monitor.CoreCLR.cs index c81f1d2ad126a6..ed6f0e2a7c391c 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Threading/Monitor.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Threading/Monitor.CoreCLR.cs @@ -231,7 +231,7 @@ public static void TryEnter(object obj, ref bool lockTaken) ArgumentNullException.ThrowIfNull(obj); - TryEnter_WithLockTaken(obj, ref lockTaken); + TryEnter_Timeout_WithLockTaken(obj, 0, ref lockTaken); } /*========================================================================= From 82a4d2ea779d1b23f7b6097297aca0008a6fce8a Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Tue, 18 Mar 2025 16:00:04 -0700 Subject: [PATCH 3/8] Fix thinko where the slow path of monitor exit was not hooked up correctly. Also hard-code the exact values for the Enter/Leave results on the native side so that the values are well defined instead of implicit. --- .../src/System/Threading/Monitor.CoreCLR.cs | 10 ++++++---- src/coreclr/vm/syncblk.h | 18 ++++++++++-------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Threading/Monitor.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Threading/Monitor.CoreCLR.cs index ed6f0e2a7c391c..5149fa734fdcc0 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Threading/Monitor.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Threading/Monitor.CoreCLR.cs @@ -43,6 +43,7 @@ public static void Enter(object obj) [MethodImpl(MethodImplOptions.InternalCall)] private static extern bool TryEnter_FastPath(object obj); + // These must match the values in syncblk.h private enum EnterHelperResult { Contention = 0, @@ -50,6 +51,7 @@ private enum EnterHelperResult UseSlowPath = 2 } + // These must match the values in syncblk.h private enum LeaveHelperAction { None = 0, Signal = 1, @@ -128,11 +130,11 @@ private static void ThrowLockTakenException() [MethodImpl(MethodImplOptions.InternalCall)] private static extern LeaveHelperAction Exit_FastPath(object obj); - [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "Monitor_Enter_Slowpath")] + [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "Monitor_Exit_Slowpath")] private static partial void Exit_Slowpath(ObjectHandleOnStack obj, LeaveHelperAction exitBehavior); [MethodImpl(MethodImplOptions.NoInlining)] - private static void Exit_Slowpath(object obj, LeaveHelperAction exitBehavior) + private static void Exit_Slowpath(LeaveHelperAction exitBehavior, object obj) { Exit_Slowpath(ObjectHandleOnStack.Create(ref obj), exitBehavior); } @@ -153,7 +155,7 @@ public static void Exit(object obj) if (exitBehavior == LeaveHelperAction.None) return; - Exit_Slowpath(obj, exitBehavior); + Exit_Slowpath(exitBehavior, obj); } // Used to implement synchronized methods on non Windows-X86 architectures @@ -171,7 +173,7 @@ internal static void ExitIfLockTaken(object obj, ref bool lockTaken) return; } - Exit_Slowpath(obj, exitBehavior); + Exit_Slowpath(exitBehavior, obj); lockTaken = false; } } diff --git a/src/coreclr/vm/syncblk.h b/src/coreclr/vm/syncblk.h index b9498ec1549999..e0ef98c3fcd153 100644 --- a/src/coreclr/vm/syncblk.h +++ b/src/coreclr/vm/syncblk.h @@ -162,18 +162,20 @@ class AwareLock friend class SyncBlock; public: + // These must match the values in Monitor.CoreCLR.cs enum class EnterHelperResult : INT32 { - Contention, - Entered, - UseSlowPath + Contention = 0, + Entered = 1, + UseSlowPath = 2 }; + // These must match the values in Monitor.CoreCLR.cs enum class LeaveHelperAction : INT32 { - None, - Signal, - Yield, - Contention, - Error, + None = 0, + Signal = 1, + Yield = 2, + Contention = 3, + Error = 4, }; private: From 5a314714493b1ad02ec0fa3458483004a46a27df Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Tue, 18 Mar 2025 16:25:42 -0700 Subject: [PATCH 4/8] Address code review feedback --- .../classlibnative/bcltype/objectnative.cpp | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/coreclr/classlibnative/bcltype/objectnative.cpp b/src/coreclr/classlibnative/bcltype/objectnative.cpp index 7fa4503d2cc43b..6198611a6d6185 100644 --- a/src/coreclr/classlibnative/bcltype/objectnative.cpp +++ b/src/coreclr/classlibnative/bcltype/objectnative.cpp @@ -211,9 +211,12 @@ extern "C" void QCALLTYPE Monitor_Enter_Slowpath(QCall::ObjectHandleOnStack objH { QCALL_CONTRACT; + BEGIN_QCALL; + GCX_COOP(); objHandle.Get()->EnterObjMonitor(); + END_QCALL; } /*********************************************************************/ @@ -267,12 +270,18 @@ extern "C" INT32 QCALLTYPE Monitor_TryEnter_Slowpath(QCall::ObjectHandleOnStack { QCALL_CONTRACT; + BOOL result = FALSE; + + BEGIN_QCALL; + GCX_COOP(); if (timeOut < -1) COMPlusThrow(kArgumentOutOfRangeException); - BOOL result = objHandle.Get()->TryEnterObjMonitor(timeOut); + result = objHandle.Get()->TryEnterObjMonitor(timeOut); + + END_QCALL; return result; } @@ -282,6 +291,8 @@ extern "C" void QCALLTYPE Monitor_Exit_Slowpath(QCall::ObjectHandleOnStack objHa { QCALL_CONTRACT; + BEGIN_QCALL; + GCX_COOP(); if (exitBehavior != AwareLock::LeaveHelperAction::Signal) @@ -296,6 +307,7 @@ extern "C" void QCALLTYPE Monitor_Exit_Slowpath(QCall::ObjectHandleOnStack objHa if (psb != NULL) psb->QuickGetMonitor()->Signal(); } + END_QCALL; } #include From d42f4ec26cd092bb5bc69f5872e0edd13012c71c Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Wed, 19 Mar 2025 11:47:56 -0700 Subject: [PATCH 5/8] Fix missing ThrowIfNull --- .../src/System/Threading/Monitor.CoreCLR.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Threading/Monitor.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Threading/Monitor.CoreCLR.cs index 5149fa734fdcc0..fd6c6459ced446 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Threading/Monitor.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Threading/Monitor.CoreCLR.cs @@ -150,6 +150,8 @@ private static void Exit_Slowpath(LeaveHelperAction exitBehavior, object obj) =========================================================================*/ public static void Exit(object obj) { + ArgumentNullException.ThrowIfNull(obj); + LeaveHelperAction exitBehavior = Exit_FastPath(obj); if (exitBehavior == LeaveHelperAction.None) From e1199d5d72c8044ac543441ccd62fd6def10c6ec Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Wed, 19 Mar 2025 12:00:49 -0700 Subject: [PATCH 6/8] Handle TryEnter contention scenarios correctly --- .../src/System/Threading/Monitor.CoreCLR.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Threading/Monitor.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Threading/Monitor.CoreCLR.cs index fd6c6459ced446..d9c8de26cf92ff 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Threading/Monitor.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Threading/Monitor.CoreCLR.cs @@ -214,7 +214,7 @@ private static void TryEnter_Timeout_WithLockTaken(object obj, int timeout, ref lockTaken = true; return; } - else if (tryEnterResult == EnterHelperResult.Contention) + else if (millisecondsTimeout == 0 && (tryEnterResult == EnterHelperResult.Contention)) { return; } @@ -257,7 +257,7 @@ public static bool TryEnter(object obj, int millisecondsTimeout) { return true; } - else if (tryEnterResult == EnterHelperResult.Contention) + else if (millisecondsTimeout == 0 && (tryEnterResult == EnterHelperResult.Contention)) { return false; } From 426b8ff34dd3fdce60d91bf390ca150aa3b9d6f1 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Wed, 19 Mar 2025 13:08:43 -0700 Subject: [PATCH 7/8] Fix build break --- .../src/System/Threading/Monitor.CoreCLR.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Threading/Monitor.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Threading/Monitor.CoreCLR.cs index d9c8de26cf92ff..43ed0de6226c45 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Threading/Monitor.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Threading/Monitor.CoreCLR.cs @@ -204,11 +204,11 @@ public static bool TryEnter(object obj) return TryEnter_Slowpath(obj); } - private static void TryEnter_Timeout_WithLockTaken(object obj, int timeout, ref bool lockTaken) + private static void TryEnter_Timeout_WithLockTaken(object obj, int millisecondsTimeout, ref bool lockTaken) { - if (timeout >= -1) + if (millisecondsTimeout >= -1) { - EnterHelperResult tryEnterResult = TryEnter_FastPath_WithTimeout(obj, timeout); + EnterHelperResult tryEnterResult = TryEnter_FastPath_WithTimeout(obj, millisecondsTimeout); if (tryEnterResult == EnterHelperResult.Entered) { lockTaken = true; @@ -220,7 +220,7 @@ private static void TryEnter_Timeout_WithLockTaken(object obj, int timeout, ref } } - if (TryEnter_Slowpath(obj, timeout)) + if (TryEnter_Slowpath(obj, millisecondsTimeout)) { lockTaken = true; } From 3d942ff6f6046624895ac6a6c2f99870da96cd59 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Fri, 21 Mar 2025 14:00:21 -0700 Subject: [PATCH 8/8] Produce exactly the same exceptions as before, and fix formatting issue --- .../src/System/Threading/Monitor.CoreCLR.cs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Threading/Monitor.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Threading/Monitor.CoreCLR.cs index 43ed0de6226c45..05a418e2f986a0 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Threading/Monitor.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Threading/Monitor.CoreCLR.cs @@ -32,7 +32,7 @@ public static partial class Monitor =========================================================================*/ public static void Enter(object obj) { - ArgumentNullException.ThrowIfNull(obj); + ArgumentNullException.ThrowIfNull(obj, null); if (!TryEnter_FastPath(obj)) { @@ -52,7 +52,8 @@ private enum EnterHelperResult } // These must match the values in syncblk.h - private enum LeaveHelperAction { + private enum LeaveHelperAction + { None = 0, Signal = 1, Yield = 2, @@ -111,7 +112,7 @@ public static void Enter(object obj, ref bool lockTaken) if (lockTaken) ThrowLockTakenException(); - ArgumentNullException.ThrowIfNull(obj); + ArgumentNullException.ThrowIfNull(obj, null); if (!TryEnter_FastPath(obj)) { @@ -150,7 +151,7 @@ private static void Exit_Slowpath(LeaveHelperAction exitBehavior, object obj) =========================================================================*/ public static void Exit(object obj) { - ArgumentNullException.ThrowIfNull(obj); + ArgumentNullException.ThrowIfNull(obj, null); LeaveHelperAction exitBehavior = Exit_FastPath(obj); @@ -163,7 +164,7 @@ public static void Exit(object obj) // Used to implement synchronized methods on non Windows-X86 architectures internal static void ExitIfLockTaken(object obj, ref bool lockTaken) { - ArgumentNullException.ThrowIfNull(obj); + ArgumentNullException.ThrowIfNull(obj, null); if (lockTaken) { @@ -189,7 +190,7 @@ internal static void ExitIfLockTaken(object obj, ref bool lockTaken) =========================================================================*/ public static bool TryEnter(object obj) { - ArgumentNullException.ThrowIfNull(obj); + ArgumentNullException.ThrowIfNull(obj, null); EnterHelperResult tryEnterResult = TryEnter_FastPath_WithTimeout(obj, 0); if (tryEnterResult == EnterHelperResult.Entered) @@ -233,7 +234,7 @@ public static void TryEnter(object obj, ref bool lockTaken) if (lockTaken) ThrowLockTakenException(); - ArgumentNullException.ThrowIfNull(obj); + ArgumentNullException.ThrowIfNull(obj, null); TryEnter_Timeout_WithLockTaken(obj, 0, ref lockTaken); } @@ -248,7 +249,7 @@ public static void TryEnter(object obj, ref bool lockTaken) =========================================================================*/ public static bool TryEnter(object obj, int millisecondsTimeout) { - ArgumentNullException.ThrowIfNull(obj); + ArgumentNullException.ThrowIfNull(obj, null); if (millisecondsTimeout >= -1) { @@ -273,7 +274,7 @@ public static void TryEnter(object obj, int millisecondsTimeout, ref bool lockTa if (lockTaken) ThrowLockTakenException(); - ArgumentNullException.ThrowIfNull(obj); + ArgumentNullException.ThrowIfNull(obj, null); TryEnter_Timeout_WithLockTaken(obj, millisecondsTimeout, ref lockTaken); }