From 94121ed6a65609805eb5539739ca787299d9a890 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Thu, 4 Jun 2026 11:12:59 -0700 Subject: [PATCH 01/20] timeout tweak --- .../src/System/Threading/ManualResetEventSlim.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/ManualResetEventSlim.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/ManualResetEventSlim.cs index c07fb07a8328d3..bb003eb1d8bbfa 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/ManualResetEventSlim.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/ManualResetEventSlim.cs @@ -502,7 +502,6 @@ public bool Wait(int millisecondsTimeout, CancellationToken cancellationToken) // We spin briefly before falling back to allocating and/or waiting on a true event. long startTime = 0; - bool bNeedTimeoutAdjustment = false; int realMillisecondsTimeout = millisecondsTimeout; // this will be adjusted if necessary. if (millisecondsTimeout != Timeout.Infinite) @@ -514,7 +513,6 @@ public bool Wait(int millisecondsTimeout, CancellationToken cancellationToken) // decide to block in the kernel below. startTime = Environment.TickCount64; - bNeedTimeoutAdjustment = true; } // Spin @@ -549,7 +547,7 @@ public bool Wait(int millisecondsTimeout, CancellationToken cancellationToken) cancellationToken.ThrowIfCancellationRequested(); // update timeout (delays in wait commencement are due to spinning and/or spurious wakeups from other waits being canceled) - if (bNeedTimeoutAdjustment) + if (startTime != 0) { // TimeoutHelper.UpdateTimeOut returns a long but the value is capped as millisecondsTimeout is an int. realMillisecondsTimeout = (int)TimeoutHelper.UpdateTimeOut(startTime, millisecondsTimeout); From 6b561358d8ef91017b52bef624152fb1f82d8755 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Thu, 4 Jun 2026 14:11:44 -0700 Subject: [PATCH 02/20] use InterlockedIncrement --- .../System/Threading/ManualResetEventSlim.cs | 116 ++++++------------ 1 file changed, 40 insertions(+), 76 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/ManualResetEventSlim.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/ManualResetEventSlim.cs index bb003eb1d8bbfa..e7c2b1393bb0b1 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/ManualResetEventSlim.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/ManualResetEventSlim.cs @@ -44,11 +44,10 @@ public class ManualResetEventSlim : IDisposable // -- State -- // // For a packed word a uint would seem better, but Interlocked.* doesn't support them as uint isn't CLS-compliant. - private volatile int m_combinedState; // ie a uint. Used for the state items listed below. + private int m_combinedState; // ie a uint. Used for the state items listed below. // 1-bit for signalled state private const int SignalledState_BitMask = unchecked((int)0x80000000); // 1000 0000 0000 0000 0000 0000 0000 0000 - private const int SignalledState_ShiftCount = 31; // 1-bit for disposed state private const int Dispose_BitMask = unchecked((int)0x40000000); // 0100 0000 0000 0000 0000 0000 0000 0000 @@ -60,7 +59,6 @@ public class ManualResetEventSlim : IDisposable // 19-bits for m_waiters. This allows support of 512K threads waiting which should be ample private const int NumWaitersState_BitMask = unchecked((int)0x0007FFFF); // 0000 0000 0000 0111 1111 1111 1111 1111 - private const int NumWaitersState_ShiftCount = 0; private const int NumWaitersState_MaxValue = (1 << 19) - 1; // 512K-1 // ----------- // @@ -68,7 +66,7 @@ public class ManualResetEventSlim : IDisposable /// Gets the underlying object for this . /// - /// The underlying event object fore this The underlying event object for this . /// /// Accessing this property forces initialization of an underlying event object if one hasn't @@ -97,8 +95,22 @@ public WaitHandle WaitHandle /// true if the event has is set; otherwise, false. public bool IsSet { - get => 0 != ExtractStatePortion(m_combinedState, SignalledState_BitMask); - private set => UpdateStateAtomically(((value) ? 1 : 0) << SignalledState_ShiftCount, SignalledState_BitMask); + get + { + return m_combinedState < 0; + } + + private set + { + if (value) + { + Interlocked.Or(ref m_combinedState, SignalledState_BitMask); + } + else + { + Interlocked.And(ref m_combinedState, ~SignalledState_BitMask); + } + } } /// @@ -116,23 +128,21 @@ private set } } - /// - /// How many threads are waiting. - /// - private int Waiters + private void InterlockedIncrementWaiters() { - get => ExtractStatePortionAndShiftRight(m_combinedState, NumWaitersState_BitMask, NumWaitersState_ShiftCount); - set - { - // setting to <0 would indicate an internal flaw, hence Assert is appropriate. - Debug.Assert(value >= 0, "NumWaiters should never be less than zero. This indicates an internal error."); + int newWaiters = Interlocked.Increment(ref m_combinedState) & NumWaitersState_BitMask; - // it is possible for the max number of waiters to be exceeded via user-code, hence we use a real exception here. - if (value >= NumWaitersState_MaxValue) - throw new InvalidOperationException(SR.Format(SR.ManualResetEventSlim_ctor_TooManyWaiters, NumWaitersState_MaxValue)); + // it is possible for the max number of waiters to be exceeded via user-code, hence we use a real exception here. + if (newWaiters >= NumWaitersState_MaxValue) + throw new InvalidOperationException(SR.Format(SR.ManualResetEventSlim_ctor_TooManyWaiters, NumWaitersState_MaxValue)); + } - UpdateStateAtomically(value << NumWaitersState_ShiftCount, NumWaitersState_BitMask); - } + private void InterlockedDecrementWaiters() + { + int newWaiters = Interlocked.Decrement(ref m_combinedState) & NumWaitersState_BitMask; + + // setting to <0 would indicate an internal flaw, hence Assert is appropriate. + Debug.Assert(newWaiters >= 0, "NumWaiters should never be less than zero. This indicates an internal error."); } //----------------------------------------------------------------------------------- @@ -190,7 +200,8 @@ public ManualResetEventSlim(bool initialState, int spinCount) #pragma warning disable IDE0060 // Remove unused parameter spinCount, on single-threaded systems, the spin count is not used. private void Initialize(bool initialState, int spinCount) { - m_combinedState = initialState ? (1 << SignalledState_ShiftCount) : 0; + IsSet = initialState; + // the spinCount argument has been validated by the ctors. // but we now sanity check our predefined constants. Debug.Assert(DEFAULT_SPIN_SP >= 0, "Internal error - DEFAULT_SPIN_SP is outside the legal range."); @@ -213,7 +224,7 @@ private void EnsureLockObjectCreated() } /// - /// This method lazily initializes the event object. It uses CAS to guarantee that + /// This method lazily initializes the public wait object. It uses CAS to guarantee that /// many threads racing to call this at once don't result in more than one event /// being stored and used. The event will be signaled or unsignaled depending on /// the state of the thin-event itself, with synchronization taken into account. @@ -235,7 +246,7 @@ private void LazyInitializeEvent() // Now that the event is published, verify that the state hasn't changed since // we snapped the preInitializeState. Another thread could have done that // between our initial observation above and here. The barrier incurred from - // the CAS above (in addition to m_state being volatile) prevents this read + // the CAS above prevents this read // from moving earlier and being collapsed with our original one. bool currentIsSet = IsSet; if (currentIsSet != preInitializeIsSet) @@ -275,10 +286,10 @@ private void Set(bool duringCancellation) // We need to ensure that IsSet=true does not get reordered past the read of m_eventObj // This would be a legal movement according to the .NET memory model. // The code is safe as IsSet involves an Interlocked.CompareExchange which provides a full memory barrier. - IsSet = true; + long origState = Interlocked.Or(ref m_combinedState, SignalledState_BitMask); - // If there are waiting threads, we need to pulse them. - if (Waiters > 0) + // If there were waiting threads at Set time, we need to pulse them. + if ((origState & NumWaitersState_BitMask) != 0) { Debug.Assert(m_lock != null); // if waiters>0, then m_lock has already been created. lock (m_lock) @@ -563,11 +574,11 @@ public bool Wait(int millisecondsTimeout, CancellationToken cancellationToken) // If we see IsSet=false, then we are guaranteed that Set() will see that we are // waiting and will pulse the monitor correctly. - Waiters++; + InterlockedIncrementWaiters(); if (IsSet) // This check must occur after updating Waiters. { - Waiters--; // revert the increment. + InterlockedDecrementWaiters(); // revert the increment. return true; } @@ -581,7 +592,7 @@ public bool Wait(int millisecondsTimeout, CancellationToken cancellationToken) finally { // Clean up: we're done waiting. - Waiters--; + InterlockedDecrementWaiters(); } // Now just loop back around, and the right thing will happen. Either: // 1. We had a spurious wake-up due to some other wait being canceled via a different cancellationToken (rewait) @@ -656,38 +667,6 @@ private static void CancellationTokenCallback(object? obj) } } - /// - /// Private helper method for updating parts of a bit-string state value. - /// Mainly called from the IsSet and Waiters properties setters - /// - /// - /// Note: the parameter types must be int as CompareExchange cannot take a Uint - /// - /// The new value - /// The mask used to set the bits - private void UpdateStateAtomically(int newBits, int updateBitsMask) - { - SpinWait sw = default; - - Debug.Assert((newBits | updateBitsMask) == updateBitsMask, "newBits do not fall within the updateBitsMask."); - - while (true) - { - int oldState = m_combinedState; // cache the old value for testing in CAS - - // Procedure:(1) zero the updateBits. eg oldState = [11111111] flag= [00111000] newState = [11000111] - // then (2) map in the newBits. eg [11000111] newBits=00101000, newState=[11101111] - int newState = (oldState & ~updateBitsMask) | newBits; - - if (Interlocked.CompareExchange(ref m_combinedState, newState, oldState) == oldState) - { - return; - } - - sw.SpinOnce(sleep1Threshold: -1); - } - } - /// /// Private helper method - performs Mask and shift, particular helpful to extract a field from a packed word. /// eg ExtractStatePortionAndShiftRight(0x12345678, 0xFF000000, 24) => 0x12, ie extracting the top 8-bits as a simple integer @@ -704,20 +683,5 @@ private static int ExtractStatePortionAndShiftRight(int state, int mask, int rig // then convert back to int. return unchecked((int)(((uint)(state & mask)) >> rightBitShiftCount)); } - - /// - /// Performs a Mask operation, but does not perform the shift. - /// This is acceptable for boolean values for which the shift is unnecessary - /// eg (val & Mask) != 0 is an appropriate way to extract a boolean rather than using - /// ((val & Mask) >> shiftAmount) == 1 - /// - /// ?? is there a common place to put this rather than being private to MRES? - /// - /// - /// - private static int ExtractStatePortion(int state, int mask) - { - return state & mask; - } } } From 96984be4d5151e37020a50efa449dbf842adde10 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Thu, 4 Jun 2026 21:48:44 -0700 Subject: [PATCH 03/20] remove s_conditionTable --- .../src/System/Threading/Condition.cs | 5 ++ .../src/System/Threading/Lock.cs | 58 +++++++++++++++---- .../src/System/Threading/Monitor.cs | 8 +-- 3 files changed, 56 insertions(+), 15 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/Condition.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/Condition.cs index 302721b59d8097..ff77752b05b498 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/Condition.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/Condition.cs @@ -47,6 +47,11 @@ private static void ReleaseWaiterForCurrentThread(Waiter waiter) } private readonly Lock _lock; + + // When condition is installed in a Lock it takes the same field as waitEvent would. + // If waitEvent is also needed it is available through here. + internal AutoResetEvent? _waitEvent; + private Waiter? _waitersHead; private Waiter? _waitersTail; diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/Lock.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/Lock.cs index f050a4c64b9bfa..3795d597290fcb 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/Lock.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/Lock.cs @@ -52,7 +52,39 @@ public sealed class Lock private short _spinCount; private ushort _waiterStartTimeMs; - private AutoResetEvent? _waitEvent; + + private object? _waitEventOrCondition; + private AutoResetEvent? WaitEvent + { + get + { + object? weoc = _waitEventOrCondition; + if (weoc is Condition c) + return c._waitEvent; + + return (AutoResetEvent?)weoc; + } + } + + internal Condition GetOrCreateCondition() + { + // The loop terminates as _waitEventOrCondition has limited number of + // state transitions with no cycles. + while (true) + { + object? weoc = _waitEventOrCondition; + if (weoc is Condition c) + return c; + + Condition newCondition = new Condition(this); + newCondition._waitEvent = WaitEvent; + + if (Interlocked.CompareExchange(ref _waitEventOrCondition, newCondition, weoc) == weoc) + { + return newCondition; + } + } + } /// /// Initializes a new instance of the class. @@ -513,7 +545,8 @@ internal int TryEnterSlow(int timeoutMs, int currentThreadId) NativeRuntimeEventSource.Log.IsEnabled( EventLevel.Informational, NativeRuntimeEventSource.Keywords.ContentionKeyword); - AutoResetEvent waitEvent = _waitEvent ?? CreateWaitEvent(areContentionEventsEnabled); + + AutoResetEvent waitEvent = WaitEvent ?? CreateWaitEvent(areContentionEventsEnabled); if (State.TryLockBeforeWait(this)) { // Lock was acquired and a waiter was not registered @@ -652,8 +685,13 @@ private bool ShouldStopPreemptingWaiters private AutoResetEvent CreateWaitEvent(bool areContentionEventsEnabled) { var newWaitEvent = new AutoResetEvent(false); - AutoResetEvent? waitEventBeforeUpdate = Interlocked.CompareExchange(ref _waitEvent, newWaitEvent, null); - if (waitEventBeforeUpdate == null) + object? weocBeforeUpdate = Interlocked.CompareExchange(ref _waitEventOrCondition, newWaitEvent, null); + if (weocBeforeUpdate is Condition c) + { + weocBeforeUpdate = Interlocked.CompareExchange(ref c._waitEvent, newWaitEvent, null); + } + + if (weocBeforeUpdate == null) { // Also check NativeRuntimeEventSource.Log.IsEnabled() to enable trimming if (areContentionEventsEnabled && NativeRuntimeEventSource.Log.IsEnabled()) @@ -665,7 +703,7 @@ private AutoResetEvent CreateWaitEvent(bool areContentionEventsEnabled) } newWaitEvent.Dispose(); - return waitEventBeforeUpdate; + return (AutoResetEvent)weocBeforeUpdate; } [MethodImpl(MethodImplOptions.NoInlining)] @@ -674,8 +712,8 @@ private void SignalWaiterIfNecessary(State state) if (State.TrySetIsWaiterSignaledToWake(this, state)) { // Signal a waiter to wake - Debug.Assert(_waitEvent != null); - bool signaled = _waitEvent.Set(); + Debug.Assert(WaitEvent != null); + bool signaled = WaitEvent.Set(); Debug.Assert(signaled); } } @@ -695,14 +733,14 @@ public bool IsHeldByCurrentThread } internal static long ContentionCount => s_contentionCount; - internal void Dispose() => _waitEvent?.Dispose(); + internal void Dispose() => WaitEvent?.Dispose(); internal nint LockIdForEvents { get { - Debug.Assert(_waitEvent != null); - return _waitEvent.SafeWaitHandle.DangerousGetHandle(); + Debug.Assert(WaitEvent != null); + return WaitEvent.SafeWaitHandle.DangerousGetHandle(); } } diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/Monitor.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/Monitor.cs index 0eaf0a79c14e9c..9910b6e8af1afa 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/Monitor.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/Monitor.cs @@ -78,17 +78,15 @@ private static void ThrowLockTakenException() throw new ArgumentException(SR.Argument_MustBeFalse, "lockTaken"); } - #region Object->Condition mapping - - private static readonly ConditionalWeakTable s_conditionTable = []; - private static readonly Func s_createCondition = (o) => new Condition(GetLockObject(o)); + #region Condition Wait/Pulse private static Condition GetCondition(object obj) { Debug.Assert( obj is not Condition, "Do not use Monitor.Pulse or Wait on a Condition instance; use the methods on Condition instead."); - return s_conditionTable.GetOrAdd(obj, s_createCondition); + + return GetLockObject(obj).GetOrCreateCondition(); } #endregion From 53a01216ae136d115769d143f91f7dfa5fd7e8cf Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Thu, 4 Jun 2026 23:27:43 -0700 Subject: [PATCH 04/20] arg check --- .../System.Private.CoreLib/src/System/Threading/Monitor.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/Monitor.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/Monitor.cs index 9910b6e8af1afa..7d09cea7f63e14 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/Monitor.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/Monitor.cs @@ -95,7 +95,9 @@ private static Condition GetCondition(object obj) [UnsupportedOSPlatform("browser")] public static bool Wait(object obj, int millisecondsTimeout) { + ArgumentNullException.ThrowIfNull(obj); RuntimeFeature.ThrowIfMultithreadingIsNotSupported(); + return GetCondition(obj).Wait(millisecondsTimeout, obj); } From ff31a332167bcdad25ed84d4d823517c8e13ac1d Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Thu, 4 Jun 2026 23:59:13 -0700 Subject: [PATCH 05/20] shorten time under lock --- .../System/Threading/ManualResetEventSlim.cs | 33 +++++++++---------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/ManualResetEventSlim.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/ManualResetEventSlim.cs index e7c2b1393bb0b1..42f278fd54ffc2 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/ManualResetEventSlim.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/ManualResetEventSlim.cs @@ -40,7 +40,7 @@ public class ManualResetEventSlim : IDisposable private object? m_lock; // A lock used for waiting and pulsing. Lazily initialized via EnsureLockObjectCreated() - private volatile ManualResetEvent? m_eventObj; // A true Win32 event used for waiting. + private ManualResetEvent? m_eventObj; // A true Win32 event used for waiting. // -- State -- // // For a packed word a uint would seem better, but Interlocked.* doesn't support them as uint isn't CLS-compliant. @@ -549,28 +549,27 @@ public bool Wait(int millisecondsTimeout, CancellationToken cancellationToken) // We must register and unregister the token outside of the lock, to avoid deadlocks. using (cancellationToken.UnsafeRegister(s_cancellationTokenCallback, this)) { - lock (m_lock) + // Loop to cope with spurious wakeups from other waits being canceled + while (!IsSet) { - // Loop to cope with spurious wakeups from other waits being canceled - while (!IsSet) - { - // If our token was canceled, we must throw and exit. - cancellationToken.ThrowIfCancellationRequested(); + // If our token was canceled, we must throw and exit. + cancellationToken.ThrowIfCancellationRequested(); - // update timeout (delays in wait commencement are due to spinning and/or spurious wakeups from other waits being canceled) - if (startTime != 0) - { - // TimeoutHelper.UpdateTimeOut returns a long but the value is capped as millisecondsTimeout is an int. - realMillisecondsTimeout = (int)TimeoutHelper.UpdateTimeOut(startTime, millisecondsTimeout); - if (realMillisecondsTimeout <= 0) - return false; - } + // update timeout (delays in wait commencement are due to spinning and/or spurious wakeups from other waits being canceled) + if (startTime != 0) + { + // TimeoutHelper.UpdateTimeOut returns a long but the value is capped as millisecondsTimeout is an int. + realMillisecondsTimeout = (int)TimeoutHelper.UpdateTimeOut(startTime, millisecondsTimeout); + if (realMillisecondsTimeout <= 0) + return false; + } + lock (m_lock) + { // There is a race condition that Set will fail to see that there are waiters as Set does not take the lock, // so after updating waiters, we must check IsSet again. // Also, we must ensure there cannot be any reordering of the assignment to Waiters and the - // read from IsSet. This is guaranteed as Waiters{set;} involves an Interlocked.CompareExchange - // operation which provides a full memory barrier. + // read from IsSet. This is guaranteed as InterlockedIncrementWaiters is a full memory barrier. // If we see IsSet=false, then we are guaranteed that Set() will see that we are // waiting and will pulse the monitor correctly. From b82d01928b1eb32f5be075a5fc4d88883e4b7f03 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Fri, 5 Jun 2026 00:21:34 -0700 Subject: [PATCH 06/20] Add Wait/Pulse on Lock --- .../src/System/Threading/Lock.cs | 17 +++++++++++++++++ .../src/System/Threading/Monitor.cs | 18 +++--------------- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/Lock.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/Lock.cs index 3795d597290fcb..8a9ce9c4235c7d 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/Lock.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/Lock.cs @@ -965,6 +965,23 @@ private static short DetermineMinSpinCountForAdaptiveSpin() return (short)-adaptiveSpinPeriod; } + internal bool Wait(int millisecondsTimeout) + { +#pragma warning disable CS9216 // A value of type 'System.Threading.Lock' converted to a different type will use likely unintended monitor-based locking in 'lock' statement. + return GetOrCreateCondition().Wait(millisecondsTimeout, this); +#pragma warning restore CS9216 // A value of type 'System.Threading.Lock' converted to a different type will use likely unintended monitor-based locking in 'lock' statement. + } + + internal void Pulse() + { + GetOrCreateCondition().SignalOne(); + } + + internal void PulseAll() + { + GetOrCreateCondition().SignalAll(); + } + private struct State : IEquatable { // Layout constants for Lock._state diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/Monitor.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/Monitor.cs index 7d09cea7f63e14..7b347330e27300 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/Monitor.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/Monitor.cs @@ -78,18 +78,6 @@ private static void ThrowLockTakenException() throw new ArgumentException(SR.Argument_MustBeFalse, "lockTaken"); } - #region Condition Wait/Pulse - - private static Condition GetCondition(object obj) - { - Debug.Assert( - obj is not Condition, - "Do not use Monitor.Pulse or Wait on a Condition instance; use the methods on Condition instead."); - - return GetLockObject(obj).GetOrCreateCondition(); - } - #endregion - #region Public Wait/Pulse methods [UnsupportedOSPlatform("browser")] @@ -98,21 +86,21 @@ public static bool Wait(object obj, int millisecondsTimeout) ArgumentNullException.ThrowIfNull(obj); RuntimeFeature.ThrowIfMultithreadingIsNotSupported(); - return GetCondition(obj).Wait(millisecondsTimeout, obj); + return GetLockObject(obj).Wait(millisecondsTimeout); } public static void Pulse(object obj) { ArgumentNullException.ThrowIfNull(obj); - GetCondition(obj).SignalOne(); + GetLockObject(obj).Pulse(); } public static void PulseAll(object obj) { ArgumentNullException.ThrowIfNull(obj); - GetCondition(obj).SignalAll(); + GetLockObject(obj).PulseAll(); } #endregion From 32f66dc20feffb328b69074a9f417f2054e0fbbd Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Fri, 5 Jun 2026 00:45:18 -0700 Subject: [PATCH 07/20] use Lock directly in mres --- .../System/Threading/ManualResetEventSlim.cs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/ManualResetEventSlim.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/ManualResetEventSlim.cs index 42f278fd54ffc2..0a2a654f17480e 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/ManualResetEventSlim.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/ManualResetEventSlim.cs @@ -37,8 +37,10 @@ public class ManualResetEventSlim : IDisposable // These are the default spin counts we use on single-proc and MP machines. private const int DEFAULT_SPIN_SP = 1; - private object? m_lock; // A lock used for waiting and pulsing. Lazily initialized via EnsureLockObjectCreated() + // We are not using thin/object lock here since our use pattern makes it nearly certain that + // a thin lock would be inflated. + private Lock? m_lock; private ManualResetEvent? m_eventObj; // A true Win32 event used for waiting. @@ -219,7 +221,7 @@ private void EnsureLockObjectCreated() { if (m_lock is null) { - Interlocked.CompareExchange(ref m_lock, new object(), null); // failure is benign. Someone else set the value. + Interlocked.CompareExchange(ref m_lock, new Lock(), null); // failure is benign. Someone else set the value. } } @@ -292,9 +294,9 @@ private void Set(bool duringCancellation) if ((origState & NumWaitersState_BitMask) != 0) { Debug.Assert(m_lock != null); // if waiters>0, then m_lock has already been created. - lock (m_lock) + using (m_lock.EnterScope()) { - Monitor.PulseAll(m_lock); + m_lock.PulseAll(); } } @@ -564,7 +566,7 @@ public bool Wait(int millisecondsTimeout, CancellationToken cancellationToken) return false; } - lock (m_lock) + using (m_lock.EnterScope()) { // There is a race condition that Set will fail to see that there are waiters as Set does not take the lock, // so after updating waiters, we must check IsSet again. @@ -585,7 +587,7 @@ public bool Wait(int millisecondsTimeout, CancellationToken cancellationToken) try { // ** the actual wait ** - if (!Monitor.Wait(m_lock, realMillisecondsTimeout)) + if (!m_lock.Wait(realMillisecondsTimeout)) return false; // return immediately if the timeout has expired. } finally @@ -660,9 +662,9 @@ private static void CancellationTokenCallback(object? obj) Debug.Assert(obj is ManualResetEventSlim, "Expected a ManualResetEventSlim"); ManualResetEventSlim mre = (ManualResetEventSlim)obj; Debug.Assert(mre.m_lock != null); // the lock should have been created before this callback is registered for use. - lock (mre.m_lock) + using (mre.m_lock.EnterScope()) { - Monitor.PulseAll(mre.m_lock); // awaken all waiters + mre.m_lock.PulseAll(); // awaken all waiters } } From f6d83c4fb962f1199a772d80e79b83f77d57c5a6 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Fri, 5 Jun 2026 00:52:52 -0700 Subject: [PATCH 08/20] revert s_conditionTable for now to make build happy --- .../System.Private.CoreLib/src/System/Threading/Monitor.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/Monitor.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/Monitor.cs index 7b347330e27300..2d423ef2c0c592 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/Monitor.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/Monitor.cs @@ -78,6 +78,13 @@ private static void ThrowLockTakenException() throw new ArgumentException(SR.Argument_MustBeFalse, "lockTaken"); } + #region Object->Condition mapping +#pragma warning disable CA1823 // Avoid unused private fields + // TODO: this is unused, except in DacDbiInterfaceImpl::EnumerateMonitorEventWaitList, fix and remove + private static readonly ConditionalWeakTable s_conditionTable = []; +#pragma warning restore CA1823 // Avoid unused private fields + #endregion + #region Public Wait/Pulse methods [UnsupportedOSPlatform("browser")] From 1ff55656170f16cc93a3079ea9b66aa866509a9c Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Fri, 5 Jun 2026 01:17:48 -0700 Subject: [PATCH 09/20] comment --- .../System.Private.CoreLib/src/System/Threading/Monitor.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/Monitor.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/Monitor.cs index 2d423ef2c0c592..07a3d3ae373ba2 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/Monitor.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/Monitor.cs @@ -80,7 +80,7 @@ private static void ThrowLockTakenException() #region Object->Condition mapping #pragma warning disable CA1823 // Avoid unused private fields - // TODO: this is unused, except in DacDbiInterfaceImpl::EnumerateMonitorEventWaitList, fix and remove + // TODO: this is unused, except in DacDbiInterfaceImpl::EnumerateMonitorEventWaitList, fix that and remove private static readonly ConditionalWeakTable s_conditionTable = []; #pragma warning restore CA1823 // Avoid unused private fields #endregion From 00425d84a421719166114cee68448b0f47b95931 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Fri, 5 Jun 2026 09:48:59 -0700 Subject: [PATCH 10/20] fix for dacdbiimpl --- src/coreclr/debug/daccess/dacdbiimpl.cpp | 24 +++++++++++++------ src/coreclr/vm/corelib.h | 2 +- .../src/System/Threading/Monitor.cs | 7 ------ 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/coreclr/debug/daccess/dacdbiimpl.cpp b/src/coreclr/debug/daccess/dacdbiimpl.cpp index db9423528f089f..54fe1dffded2f5 100644 --- a/src/coreclr/debug/daccess/dacdbiimpl.cpp +++ b/src/coreclr/debug/daccess/dacdbiimpl.cpp @@ -29,7 +29,6 @@ #endif // FEATURE_COMINTEROP #include "request_common.h" -#include "conditionalweaktable.h" #ifndef USE_DAC_TABLE_RVA extern "C" bool TryGetSymbol(ICorDebugDataTarget* dataTarget, uint64_t baseAddress, const char* symbolName, uint64_t* symbolAddress); @@ -6100,15 +6099,26 @@ HRESULT STDMETHODCALLTYPE DacDbiInterfaceImpl::EnumerateMonitorEventWaitList(VMP if(psb == NULL) return hr; - FieldDesc* pConditionTableField = (&g_CoreLib)->GetField(FIELD__MONITOR__CONDITION_TABLE); - CONDITIONAL_WEAK_TABLE_REF conditionTable = *(DPTR(CONDITIONAL_WEAK_TABLE_REF))PTR_TO_TADDR(pConditionTableField->GetStaticAddressHandle(pConditionTableField->GetBase())); + // The managed Lock (if any) for this object is referenced from the sync block. + // If there is no Lock, there can be no Condition installed on it and therefore no waiters. + OBJECTHANDLE lockHandle = psb->GetLockIfExists(); + if (lockHandle == (OBJECTHANDLE)NULL) + return hr; + OBJECTREF lockObj = ObjectFromHandle(lockHandle); + if (lockObj == NULL) + return hr; - OBJECTREF condition = NULL; - if (!conditionTable->TryGetValue(OBJECTREF(pObj), &condition)) - { + // Lock._waitEventOrCondition holds either an AutoResetEvent or a Condition. Only when a + // Condition has been installed does the Lock have a Monitor wait list to enumerate. + FieldDesc* pWaitEventOrConditionField = (&g_CoreLib)->GetField(FIELD__LOCK__WAIT_EVENT_OR_CONDITION); + OBJECTREF condition = pWaitEventOrConditionField->GetRefValue(lockObj); + if (condition == NULL) + return hr; + + PTR_MethodTable pConditionMT = CoreLibBinder::GetExistingClass(CLASS__CONDITION); + if (condition->GetMethodTable() != pConditionMT) return hr; - } MapSHash waiterToThreadMap; FieldDesc* pConditionWaiterField = (&g_CoreLib)->GetField(FIELD__CONDITION__WAITERS_HEAD); diff --git a/src/coreclr/vm/corelib.h b/src/coreclr/vm/corelib.h index cc515fd5f610e8..9780fbacbdb913 100644 --- a/src/coreclr/vm/corelib.h +++ b/src/coreclr/vm/corelib.h @@ -626,7 +626,6 @@ DEFINE_CLASS(OLE_AUT_BINDER, System, OleAutBinder) END_ILLINK_FEATURE_SWITCH() DEFINE_CLASS(MONITOR, Threading, Monitor) -DEFINE_FIELD(MONITOR, CONDITION_TABLE, s_conditionTable) DEFINE_METHOD(MONITOR, SYNCHRONIZED_METHOD_ENTER, SynchronizedMethodEnter, SM_Obj_RefBool_RetVoid) DEFINE_METHOD(MONITOR, SYNCHRONIZED_METHOD_EXIT, SynchronizedMethodExit, SM_Obj_RefBool_RetVoid) @@ -634,6 +633,7 @@ DEFINE_CLASS(LOCK, Threading, Lock) DEFINE_FIELD(LOCK, OWNING_THREAD_ID, _owningThreadId) DEFINE_FIELD(LOCK, STATE, _state) DEFINE_FIELD(LOCK, RECURSION_COUNT, _recursionCount) +DEFINE_FIELD(LOCK, WAIT_EVENT_OR_CONDITION,_waitEventOrCondition) DEFINE_METHOD(LOCK, CTOR, .ctor, IM_RetVoid) DEFINE_METHOD(LOCK, INITIALIZE_FOR_MONITOR, InitializeForMonitor, SM_PtrLock_Int_UInt_PtrException_RetVoid) diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/Monitor.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/Monitor.cs index 07a3d3ae373ba2..7b347330e27300 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/Monitor.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/Monitor.cs @@ -78,13 +78,6 @@ private static void ThrowLockTakenException() throw new ArgumentException(SR.Argument_MustBeFalse, "lockTaken"); } - #region Object->Condition mapping -#pragma warning disable CA1823 // Avoid unused private fields - // TODO: this is unused, except in DacDbiInterfaceImpl::EnumerateMonitorEventWaitList, fix that and remove - private static readonly ConditionalWeakTable s_conditionTable = []; -#pragma warning restore CA1823 // Avoid unused private fields - #endregion - #region Public Wait/Pulse methods [UnsupportedOSPlatform("browser")] From 7a52c76d3b04f43bb69c48f1a4661a78c546c448 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Fri, 5 Jun 2026 10:07:44 -0700 Subject: [PATCH 11/20] trivial cleanups --- .../src/System/Threading/Condition.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/Condition.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/Condition.cs index ff77752b05b498..606088683d0183 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/Condition.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/Condition.cs @@ -57,7 +57,7 @@ private static void ReleaseWaiterForCurrentThread(Waiter waiter) internal Lock AssociatedLock => _lock; - private unsafe void AssertIsInList(Waiter waiter) + private void AssertIsInList(Waiter waiter) { Debug.Assert(_waitersHead != null && _waitersTail != null); Debug.Assert((_waitersHead == waiter) == (waiter.prev == null)); @@ -69,7 +69,7 @@ private unsafe void AssertIsInList(Waiter waiter) Debug.Fail("Waiter is not in the waiter list"); } - private unsafe void AssertIsNotInList(Waiter waiter) + private void AssertIsNotInList(Waiter waiter) { Debug.Assert(waiter.next == null && waiter.prev == null); Debug.Assert((_waitersHead == null) == (_waitersTail == null)); @@ -79,7 +79,7 @@ private unsafe void AssertIsNotInList(Waiter waiter) Debug.Fail("Waiter is in the waiter list, but should not be"); } - private unsafe void AddWaiter(Waiter waiter) + private void AddWaiter(Waiter waiter) { Debug.Assert(_lock.IsHeldByCurrentThread); AssertIsNotInList(waiter); @@ -92,7 +92,7 @@ private unsafe void AddWaiter(Waiter waiter) _waitersHead ??= waiter; } - private unsafe void RemoveWaiter(Waiter waiter) + private void RemoveWaiter(Waiter waiter) { Debug.Assert(_lock.IsHeldByCurrentThread); AssertIsInList(waiter); @@ -119,7 +119,7 @@ public Condition(Lock @lock) _lock = @lock; } - public unsafe bool Wait(int millisecondsTimeout, object associatedObjectForMonitorWait) + public bool Wait(int millisecondsTimeout, object associatedObjectForMonitorWait) { ArgumentOutOfRangeException.ThrowIfLessThan(millisecondsTimeout, -1); @@ -154,7 +154,7 @@ public unsafe bool Wait(int millisecondsTimeout, object associatedObjectForMonit else if (!success) { // - // The wait timed out, but we were signalled before we could reacquire the lock. + // The wait timed out, but we were signaled before we could reacquire the lock. // Since WaitOne timed out, it didn't trigger the auto-reset of the AutoResetEvent. // So, we need to manually reset the event. // @@ -168,7 +168,7 @@ public unsafe bool Wait(int millisecondsTimeout, object associatedObjectForMonit return waiter.signalled; } - public unsafe void SignalAll() + public void SignalAll() { if (!_lock.IsHeldByCurrentThread) throw new SynchronizationLockException(); @@ -177,7 +177,7 @@ public unsafe void SignalAll() SignalOne(); } - public unsafe void SignalOne() + public void SignalOne() { if (!_lock.IsHeldByCurrentThread) throw new SynchronizationLockException(); From a1eb30928e1af66238e48bbed7bf59c54586d6e6 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Fri, 5 Jun 2026 11:12:41 -0700 Subject: [PATCH 12/20] removed Waiter.signalled --- .../src/System/Threading/Condition.cs | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/Condition.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/Condition.cs index 606088683d0183..7f7711bfe16a91 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/Condition.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/Condition.cs @@ -15,7 +15,6 @@ internal sealed class Waiter public Waiter? next; public Waiter? prev; public AutoResetEvent ev = new AutoResetEvent(false); - public bool signalled; } [ThreadStatic] @@ -36,7 +35,6 @@ private static Waiter GetWaiterForCurrentThread() waiter = new Waiter(); } - waiter.signalled = false; return waiter; } @@ -49,7 +47,7 @@ private static void ReleaseWaiterForCurrentThread(Waiter waiter) private readonly Lock _lock; // When condition is installed in a Lock it takes the same field as waitEvent would. - // If waitEvent is also needed it is available through here. + // If waitEvent is also needed, it is available through here. internal AutoResetEvent? _waitEvent; private Waiter? _waitersHead; @@ -79,6 +77,13 @@ private void AssertIsNotInList(Waiter waiter) Debug.Fail("Waiter is in the waiter list, but should not be"); } + // Returns true if the waiter cannot be possibly in the list. + // (i.e. not reachable via _waitersHead) + internal bool NotInList(Waiter waiter) + { + return _waitersHead != waiter && waiter.prev == null; + } + private void AddWaiter(Waiter waiter) { Debug.Assert(_lock.IsHeldByCurrentThread); @@ -133,6 +138,7 @@ public bool Wait(int millisecondsTimeout, object associatedObjectForMonitorWait) uint recursionCount = _lock.ExitAll(); bool success = false; + bool wasSignaled; try { success = @@ -147,7 +153,9 @@ public bool Wait(int millisecondsTimeout, object associatedObjectForMonitorWait) _lock.Reenter(recursionCount); Debug.Assert(_lock.IsHeldByCurrentThread); - if (!waiter.signalled) + // If the waiter is still in the list, it was not signaled. + wasSignaled = NotInList(waiter); + if (!wasSignaled) { RemoveWaiter(waiter); } @@ -165,7 +173,7 @@ public bool Wait(int millisecondsTimeout, object associatedObjectForMonitorWait) ReleaseWaiterForCurrentThread(waiter); } - return waiter.signalled; + return wasSignaled; } public void SignalAll() @@ -186,7 +194,6 @@ public void SignalOne() if (waiter != null) { RemoveWaiter(waiter); - waiter.signalled = true; waiter.ev.Set(); } } From e62dff3d1d206916ce72ac8d55ad70d7dcdf280d Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Fri, 5 Jun 2026 11:26:27 -0700 Subject: [PATCH 13/20] pwer tweaks --- .../System.Private.CoreLib/src/System/Threading/Condition.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/Condition.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/Condition.cs index 7f7711bfe16a91..b4fbcda5f2992e 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/Condition.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/Condition.cs @@ -35,6 +35,7 @@ private static Waiter GetWaiterForCurrentThread() waiter = new Waiter(); } + Debug.Assert(waiter.next is null && waiter.prev is null); return waiter; } @@ -55,6 +56,7 @@ private static void ReleaseWaiterForCurrentThread(Waiter waiter) internal Lock AssociatedLock => _lock; + [Conditional("DEBUG")] private void AssertIsInList(Waiter waiter) { Debug.Assert(_waitersHead != null && _waitersTail != null); @@ -67,6 +69,7 @@ private void AssertIsInList(Waiter waiter) Debug.Fail("Waiter is not in the waiter list"); } + [Conditional("DEBUG")] private void AssertIsNotInList(Waiter waiter) { Debug.Assert(waiter.next == null && waiter.prev == null); From 41526135fcc3148f52cd7872b10e7e06d1de16ad Mon Sep 17 00:00:00 2001 From: vsadov Date: Fri, 5 Jun 2026 13:53:35 -0700 Subject: [PATCH 14/20] some perf tweaks --- .../src/System/Threading/Condition.cs | 42 +++++++++++++++---- 1 file changed, 34 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/Condition.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/Condition.cs index b4fbcda5f2992e..1101821aaa00a3 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/Condition.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/Condition.cs @@ -14,7 +14,7 @@ internal sealed class Waiter { public Waiter? next; public Waiter? prev; - public AutoResetEvent ev = new AutoResetEvent(false); + public readonly AutoResetEvent ev = new AutoResetEvent(false); } [ThreadStatic] @@ -92,12 +92,17 @@ private void AddWaiter(Waiter waiter) Debug.Assert(_lock.IsHeldByCurrentThread); AssertIsNotInList(waiter); - waiter.prev = _waitersTail; - waiter.prev?.next = waiter; - + Waiter? tail = _waitersTail; + waiter.prev = tail; + if (tail is null) + { + _waitersHead = waiter; + } + else + { + tail.next = waiter; + } _waitersTail = waiter; - - _waitersHead ??= waiter; } private void RemoveWaiter(Waiter waiter) @@ -184,8 +189,29 @@ public void SignalAll() if (!_lock.IsHeldByCurrentThread) throw new SynchronizationLockException(); - while (_waitersHead != null) - SignalOne(); + Waiter? waiter = _waitersHead; + if (waiter is null) + { + return; + } + + // Detach the entire waiter list in one operation, then walk it and signal each waiter. + // Per-waiter prev/next must be cleared BEFORE calling ev.Set() so that the woken thread + // observes the waiter as not in the list (see NotInList) and the cached Waiter is clean. + // Woken threads cannot make progress until the caller releases _lock, so it is safe to + // continue walking the detached list after signaling each waiter. + _waitersHead = null; + _waitersTail = null; + + do + { + Waiter? next = waiter.next; + waiter.next = null; + waiter.prev = null; + AutoResetEvent ev = waiter.ev; + waiter = next; + ev.Set(); + } while (waiter is not null); } public void SignalOne() From 34189f7fb04b80708492e46893cbef5279823690 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Sat, 6 Jun 2026 15:00:17 -0700 Subject: [PATCH 15/20] couple cleanups --- .../src/System/Threading/Condition.cs | 9 +++------ .../src/System/Threading/ManualResetEventSlim.cs | 4 ++-- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/Condition.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/Condition.cs index 1101821aaa00a3..34e0834290dba5 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/Condition.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/Condition.cs @@ -1,8 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -#pragma warning disable 0420 //passing volatile fields by ref - using System.Diagnostics; using System.Diagnostics.Tracing; @@ -203,15 +201,14 @@ public void SignalAll() _waitersHead = null; _waitersTail = null; - do + while (waiter is not null) { Waiter? next = waiter.next; waiter.next = null; waiter.prev = null; - AutoResetEvent ev = waiter.ev; + waiter.ev.Set(); waiter = next; - ev.Set(); - } while (waiter is not null); + } } public void SignalOne() diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/ManualResetEventSlim.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/ManualResetEventSlim.cs index 0a2a654f17480e..b31d7de8fdbca1 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/ManualResetEventSlim.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/ManualResetEventSlim.cs @@ -39,7 +39,7 @@ public class ManualResetEventSlim : IDisposable // A lock used for waiting and pulsing. Lazily initialized via EnsureLockObjectCreated() // We are not using thin/object lock here since our use pattern makes it nearly certain that - // a thin lock would be inflated. + // a thin lock would be inflated even if we use m_lock only once. private Lock? m_lock; private ManualResetEvent? m_eventObj; // A true Win32 event used for waiting. @@ -202,7 +202,7 @@ public ManualResetEventSlim(bool initialState, int spinCount) #pragma warning disable IDE0060 // Remove unused parameter spinCount, on single-threaded systems, the spin count is not used. private void Initialize(bool initialState, int spinCount) { - IsSet = initialState; + m_combinedState = initialState ? SignalledState_BitMask : 0; // the spinCount argument has been validated by the ctors. // but we now sanity check our predefined constants. From 57420ac07c20ac7a07f957e1197c3326a19e81c2 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Sat, 6 Jun 2026 16:28:17 -0700 Subject: [PATCH 16/20] Allow internal-only use of Wait/Pulse on Lock in MONO --- .../src/System.Private.CoreLib.Shared.projitems | 4 ++-- .../src/System/Threading/ThreadBlockingInfo.cs | 4 ---- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems index 7e9355d26d52a0..ff44ef3c55770b 100644 --- a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems +++ b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems @@ -1272,7 +1272,7 @@ - + @@ -2998,4 +2998,4 @@ - + \ No newline at end of file diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/ThreadBlockingInfo.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/ThreadBlockingInfo.cs index 15155433ed50d7..b371ef5094fbe9 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/ThreadBlockingInfo.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/ThreadBlockingInfo.cs @@ -86,11 +86,9 @@ public int LockOwnerManagedThreadId // the getter may be used by debuggers case ObjectKind.Lock: return ((Lock)Unsafe.AsRef(_objectPtr)).OwningManagedThreadId; -#if !MONO case ObjectKind.Condition: Debug.Assert(_objectKind == ObjectKind.Condition); return ((Condition)Unsafe.AsRef(_objectPtr)).AssociatedLock.OwningManagedThreadId; -#endif default: throw new UnreachableException(); @@ -107,9 +105,7 @@ public ref struct Scope public Scope(Lock lockObj, int timeoutMs) : this(lockObj, ObjectKind.Lock, timeoutMs) { } #pragma warning restore CS9216 -#if !MONO public Scope(Condition condition, int timeoutMs) : this(condition, ObjectKind.Condition, timeoutMs) { } -#endif private Scope(object obj, ObjectKind objectKind, int timeoutMs) { From 145490eb6d2612f895bd07625382f1596e184cf5 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Sat, 6 Jun 2026 17:14:28 -0700 Subject: [PATCH 17/20] PR feedback (more robust check in InterlockedIncrementWaiters) --- .../System/Threading/ManualResetEventSlim.cs | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/ManualResetEventSlim.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/ManualResetEventSlim.cs index b31d7de8fdbca1..75224acce482b5 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/ManualResetEventSlim.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/ManualResetEventSlim.cs @@ -132,11 +132,22 @@ private set private void InterlockedIncrementWaiters() { - int newWaiters = Interlocked.Increment(ref m_combinedState) & NumWaitersState_BitMask; - - // it is possible for the max number of waiters to be exceeded via user-code, hence we use a real exception here. - if (newWaiters >= NumWaitersState_MaxValue) - throw new InvalidOperationException(SR.Format(SR.ManualResetEventSlim_ctor_TooManyWaiters, NumWaitersState_MaxValue)); + int currentState = m_combinedState; + while (true) + { + // it is possible for the max number of waiters to be exceeded via user-code, hence we use a real exception here. + if ((currentState & NumWaitersState_BitMask) >= NumWaitersState_MaxValue) + throw new InvalidOperationException(SR.Format(SR.ManualResetEventSlim_ctor_TooManyWaiters, NumWaitersState_MaxValue)); + + // The waiters count has room for one more, so adding 1 to the full state + // cannot carry out of NumWaitersState_BitMask. + int oldState = Interlocked.CompareExchange(ref m_combinedState, currentState + 1, currentState); + if (oldState == currentState) + return; + + // Highly unlikely contention, since we increment/decrement while holding a lock, just try again. + currentState = oldState; + } } private void InterlockedDecrementWaiters() From cf9d652f350d55a4a3091990f49a357ade836fa4 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Sat, 6 Jun 2026 17:26:53 -0700 Subject: [PATCH 18/20] updated comments --- .../src/System/Threading/ManualResetEventSlim.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/ManualResetEventSlim.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/ManualResetEventSlim.cs index 75224acce482b5..21a4b729757f47 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/ManualResetEventSlim.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/ManualResetEventSlim.cs @@ -296,12 +296,11 @@ public void Set() /// The object has been canceled. private void Set(bool duringCancellation) { - // We need to ensure that IsSet=true does not get reordered past the read of m_eventObj - // This would be a legal movement according to the .NET memory model. - // The code is safe as IsSet involves an Interlocked.CompareExchange which provides a full memory barrier. + // We need to ensure that the state change happens before we read the m_eventObj. + // Interlocked guarantees that. long origState = Interlocked.Or(ref m_combinedState, SignalledState_BitMask); - // If there were waiting threads at Set time, we need to pulse them. + // If there were waiting threads when we flipped the state, we need to pulse them. if ((origState & NumWaitersState_BitMask) != 0) { Debug.Assert(m_lock != null); // if waiters>0, then m_lock has already been created. From 8d680a41b983dd22d6c579269574dc89a3131b7f Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Sat, 6 Jun 2026 18:09:39 -0700 Subject: [PATCH 19/20] PR feedback --- .../src/System/Threading/ManualResetEventSlim.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/ManualResetEventSlim.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/ManualResetEventSlim.cs index 21a4b729757f47..319afdd30951e4 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/ManualResetEventSlim.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/ManualResetEventSlim.cs @@ -99,9 +99,10 @@ public bool IsSet { get { - return m_combinedState < 0; + // Volatile to make sure checking IsSet is not optimized away or reordered + // when called as a public API. + return Volatile.Read(ref m_combinedState) < 0; } - private set { if (value) From 0c0f58a566b41c536b651c0c408ec7d33c3759e6 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Sat, 6 Jun 2026 19:26:58 -0700 Subject: [PATCH 20/20] PR feedback. --- .../System.Private.CoreLib/src/System/Threading/Lock.cs | 7 ++++++- .../src/System/Threading/ManualResetEventSlim.cs | 6 +++--- .../System.Private.CoreLib/src/System/Threading/Monitor.cs | 2 +- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/Lock.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/Lock.cs index 8a9ce9c4235c7d..7ea7244e1f99fa 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/Lock.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/Lock.cs @@ -968,10 +968,15 @@ private static short DetermineMinSpinCountForAdaptiveSpin() internal bool Wait(int millisecondsTimeout) { #pragma warning disable CS9216 // A value of type 'System.Threading.Lock' converted to a different type will use likely unintended monitor-based locking in 'lock' statement. - return GetOrCreateCondition().Wait(millisecondsTimeout, this); + return Wait(millisecondsTimeout, this); #pragma warning restore CS9216 // A value of type 'System.Threading.Lock' converted to a different type will use likely unintended monitor-based locking in 'lock' statement. } + internal bool Wait(int millisecondsTimeout, object associatedObject) + { + return GetOrCreateCondition().Wait(millisecondsTimeout, associatedObject); + } + internal void Pulse() { GetOrCreateCondition().SignalOne(); diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/ManualResetEventSlim.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/ManualResetEventSlim.cs index 319afdd30951e4..0ececc08ec100b 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/ManualResetEventSlim.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/ManualResetEventSlim.cs @@ -153,10 +153,10 @@ private void InterlockedIncrementWaiters() private void InterlockedDecrementWaiters() { - int newWaiters = Interlocked.Decrement(ref m_combinedState) & NumWaitersState_BitMask; - // setting to <0 would indicate an internal flaw, hence Assert is appropriate. - Debug.Assert(newWaiters >= 0, "NumWaiters should never be less than zero. This indicates an internal error."); + Debug.Assert((m_combinedState & NumWaitersState_BitMask) != 0, "NumWaiters should never be less than zero. This indicates an internal error."); + + Interlocked.Decrement(ref m_combinedState); } //----------------------------------------------------------------------------------- diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/Monitor.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/Monitor.cs index 7b347330e27300..3d590d94ba1f26 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/Monitor.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/Monitor.cs @@ -86,7 +86,7 @@ public static bool Wait(object obj, int millisecondsTimeout) ArgumentNullException.ThrowIfNull(obj); RuntimeFeature.ThrowIfMultithreadingIsNotSupported(); - return GetLockObject(obj).Wait(millisecondsTimeout); + return GetLockObject(obj).Wait(millisecondsTimeout, obj); } public static void Pulse(object obj)