Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,77 @@ public static partial class Monitor
**
** Exceptions: ArgumentNullException if object is null.
=========================================================================*/
public static void Enter(object obj)
{
ArgumentNullException.ThrowIfNull(obj, null);

if (!TryEnter_FastPath(obj))
{
Enter_Slowpath(obj);
}
}

[MethodImpl(MethodImplOptions.InternalCall)]
public static extern void Enter(object obj);
private static extern bool TryEnter_FastPath(object obj);

// These must match the values in syncblk.h
private enum EnterHelperResult
{
Contention = 0,
Entered = 1,
UseSlowPath = 2
}

// These must match the values in syncblk.h
private enum LeaveHelperAction
{
None = 0,
Signal = 1,
Yield = 2,
Contention = 3,
Error = 4,
};
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
};
}


[MethodImpl(MethodImplOptions.InternalCall)]
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
Expand All @@ -44,7 +112,13 @@ public static void Enter(object obj, ref bool lockTaken)
if (lockTaken)
ThrowLockTakenException();

ReliableEnter(obj, ref lockTaken);
ArgumentNullException.ThrowIfNull(obj, null);

if (!TryEnter_FastPath(obj))
{
Enter_Slowpath(obj);
}
lockTaken = true;
Debug.Assert(lockTaken);
}

Expand All @@ -55,9 +129,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_Exit_Slowpath")]
private static partial void Exit_Slowpath(ObjectHandleOnStack obj, LeaveHelperAction exitBehavior);

[MethodImpl(MethodImplOptions.NoInlining)]
private static void Exit_Slowpath(LeaveHelperAction exitBehavior, object obj)
{
Exit_Slowpath(ObjectHandleOnStack.Create(ref obj), exitBehavior);
}

/*=========================================================================
** Release the monitor lock. If one or more threads are waiting to acquire the
Expand All @@ -68,8 +149,37 @@ 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)
{
ArgumentNullException.ThrowIfNull(obj, null);

LeaveHelperAction exitBehavior = Exit_FastPath(obj);

if (exitBehavior == LeaveHelperAction.None)
return;

Exit_Slowpath(exitBehavior, obj);
}

// Used to implement synchronized methods on non Windows-X86 architectures
internal static void ExitIfLockTaken(object obj, ref bool lockTaken)
{
ArgumentNullException.ThrowIfNull(obj, null);

if (lockTaken)
{
LeaveHelperAction exitBehavior = Exit_FastPath(obj);

if (exitBehavior == LeaveHelperAction.None)
{
lockTaken = false;
return;
}

Exit_Slowpath(exitBehavior, obj);
lockTaken = false;
Comment on lines +173 to +180
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (exitBehavior == LeaveHelperAction.None)
{
lockTaken = false;
return;
}
Exit_Slowpath(exitBehavior, obj);
lockTaken = false;
if (exitBehavior != LeaveHelperAction.None)
{
Exit_Slowpath(exitBehavior, obj);
}
lockTaken = false;

}
}

/*=========================================================================
** Similar to Enter, but will never block. That is, if the current thread can
Expand All @@ -80,9 +190,41 @@ private static void ThrowLockTakenException()
=========================================================================*/
public static bool TryEnter(object obj)
{
bool lockTaken = false;
TryEnter(obj, 0, ref lockTaken);
return lockTaken;
ArgumentNullException.ThrowIfNull(obj, null);

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 millisecondsTimeout, ref bool lockTaken)
{
Copy link

Copilot AI Mar 7, 2025

Choose a reason for hiding this comment

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

The TryEnter_Timeout_WithLockTaken method does not handle cases where timeout is less than -1. It is recommended to add an explicit check and throw an ArgumentException when timeout is invalid.

Suggested change
{
{
if (timeout < -1)
{
throw new ArgumentException("Timeout must be greater than or equal to -1.", nameof(timeout));
}

Copilot uses AI. Check for mistakes.

if (millisecondsTimeout >= -1)
{
EnterHelperResult tryEnterResult = TryEnter_FastPath_WithTimeout(obj, millisecondsTimeout);
if (tryEnterResult == EnterHelperResult.Entered)
{
lockTaken = true;
return;
}
else if (millisecondsTimeout == 0 && (tryEnterResult == EnterHelperResult.Contention))
{
return;
}
}

if (TryEnter_Slowpath(obj, millisecondsTimeout))
{
lockTaken = true;
}
}

// The JIT should inline this method to allow check of lockTaken argument to be optimized out
Expand All @@ -92,7 +234,9 @@ public static void TryEnter(object obj, ref bool lockTaken)
if (lockTaken)
ThrowLockTakenException();

ReliableEnterTimeout(obj, 0, ref lockTaken);
ArgumentNullException.ThrowIfNull(obj, null);

TryEnter_Timeout_WithLockTaken(obj, 0, ref lockTaken);
}

/*=========================================================================
Expand All @@ -103,13 +247,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, null);

Copy link

Copilot AI Mar 7, 2025

Choose a reason for hiding this comment

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

The TryEnter(object, int) method does not explicitly validate that millisecondsTimeout is not less than -1. To maintain the documented contract, consider adding a check to throw ArgumentException for invalid timeout values.

Suggested change
if (millisecondsTimeout < -1)
{
throw new ArgumentException("Timeout must be greater than or equal to -1.", nameof(millisecondsTimeout));
}

Copilot uses AI. Check for mistakes.

if (millisecondsTimeout >= -1)
{
EnterHelperResult tryEnterResult = TryEnter_FastPath_WithTimeout(obj, millisecondsTimeout);
if (tryEnterResult == EnterHelperResult.Entered)
{
return true;
}
else if (millisecondsTimeout == 0 && (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
Expand All @@ -119,11 +274,10 @@ public static void TryEnter(object obj, int millisecondsTimeout, ref bool lockTa
if (lockTaken)
ThrowLockTakenException();

ReliableEnterTimeout(obj, millisecondsTimeout, ref lockTaken);
}
ArgumentNullException.ThrowIfNull(obj, null);

[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)
{
Expand Down
122 changes: 122 additions & 0 deletions src/coreclr/classlibnative/bcltype/objectnative.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -199,3 +199,125 @@ extern "C" INT64 QCALLTYPE Monitor_GetLockContentionCount()
END_QCALL;
return result;
}

//========================================================================
//
// MONITOR HELPERS
//
//========================================================================

/*********************************************************************/
extern "C" void QCALLTYPE Monitor_Enter_Slowpath(QCall::ObjectHandleOnStack objHandle)
{
QCALL_CONTRACT;

BEGIN_QCALL;

GCX_COOP();

objHandle.Get()->EnterObjMonitor();
END_QCALL;
}

/*********************************************************************/
#include <optsmallperfcritical.h>

FCIMPL1(FC_BOOL_RET, ObjectNative::Monitor_TryEnter_FastPath, Object* obj)
{
FCALL_CONTRACT;

if (obj->TryEnterObjMonitorSpinHelper())
{
FC_RETURN_BOOL(TRUE);
}
else
{
FC_RETURN_BOOL(FALSE);
}
Comment on lines +229 to +236
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (obj->TryEnterObjMonitorSpinHelper())
{
FC_RETURN_BOOL(TRUE);
}
else
{
FC_RETURN_BOOL(FALSE);
}
FC_RETURN_BOOL(obj->TryEnterObjMonitorSpinHelper());

}
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);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should a timeOut of non-zero be handled, should it be passed down to/honored by the EnterObjMonitorHelperSpin?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, this is the fast set of spinning that waits for fractions of a millisecond or so, if you're willing to wait at all, we run the same amount of spinning for all possible timeouts.

}

return result;
}
FCIMPLEND

#include <optdefault.h>

/*********************************************************************/
extern "C" INT32 QCALLTYPE Monitor_TryEnter_Slowpath(QCall::ObjectHandleOnStack objHandle, INT32 timeOut)
{
QCALL_CONTRACT;

BOOL result = FALSE;

BEGIN_QCALL;

GCX_COOP();

if (timeOut < -1)
COMPlusThrow(kArgumentOutOfRangeException);
Comment on lines +279 to +280
Copy link
Member

Choose a reason for hiding this comment

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

I think we should assert this here and do the error handling in managed code. The Copilot for the methods also called this out.


result = objHandle.Get()->TryEnterObjMonitor(timeOut);

END_QCALL;

return result;
}

/*********************************************************************/
extern "C" void QCALLTYPE Monitor_Exit_Slowpath(QCall::ObjectHandleOnStack objHandle, AwareLock::LeaveHelperAction exitBehavior)
{
QCALL_CONTRACT;

BEGIN_QCALL;

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();
}
END_QCALL;
}

#include <optsmallperfcritical.h>
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 <optdefault.h>

8 changes: 8 additions & 0 deletions src/coreclr/classlibnative/bcltype/objectnative.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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_
Loading
Loading