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 50eb2a1e263619..302721b59d8097 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/Condition.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/Condition.cs @@ -21,13 +21,31 @@ internal sealed class Waiter [ThreadStatic] private static Waiter? t_waiterForCurrentThread; + // Takes the cached Waiter for this thread (or allocates a new one) and removes the + // current wait's cached Waiter from the thread-static so that any reentrant + // Monitor.Wait (for example, from a SynchronizationContext message pump) gets its own Waiter with a distinct AutoResetEvent. private static Waiter GetWaiterForCurrentThread() { - Waiter waiter = t_waiterForCurrentThread ??= new Waiter(); + Waiter? waiter = t_waiterForCurrentThread; + if (waiter is not null) + { + t_waiterForCurrentThread = null; + } + else + { + waiter = new Waiter(); + } + waiter.signalled = false; return waiter; } + private static void ReleaseWaiterForCurrentThread(Waiter waiter) + { + // Return the waiter to the thread-static cache for reuse. + t_waiterForCurrentThread = waiter; + } + private readonly Lock _lock; private Waiter? _waitersHead; private Waiter? _waitersTail; @@ -139,6 +157,7 @@ public unsafe bool Wait(int millisecondsTimeout, object associatedObjectForMonit } AssertIsNotInList(waiter); + ReleaseWaiterForCurrentThread(waiter); } return waiter.signalled; diff --git a/src/libraries/System.Threading/tests/MonitorTests.cs b/src/libraries/System.Threading/tests/MonitorTests.cs index 249fa2722154b6..703f1a345fad16 100644 --- a/src/libraries/System.Threading/tests/MonitorTests.cs +++ b/src/libraries/System.Threading/tests/MonitorTests.cs @@ -510,5 +510,70 @@ public static void InterruptWaitTest() waitForThread(); } } + + // Validates that reentrant Monitor.Wait calls from a SynchronizationContext + // message pump do not steal each other's pulse signals. + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsMultithreadingSupported))] + [ActiveIssue("https://github.com/dotnet/runtime/issues/123173", TestRuntimes.Mono)] + public static void ReentrantWaitFromSyncContextTest() + { + // Since we set the SynchronizationContext for the current thread, we run + // this test in a background thread to avoid affecting other tests. + ThreadTestHelpers.RunTestInBackgroundThread(() => + { + object lockA = new(); + object lockB = new(); + bool innerWaitResult = true; // should become false (lockB is never pulsed) + + var pumpCtx = new ReentrantWaitSyncContext(() => + { + lock (lockB) + { + innerWaitResult = Monitor.Wait(lockB, 500); + } + }); + SynchronizationContext.SetSynchronizationContext(pumpCtx); + + var t = new Thread(() => + { + Thread.Sleep(100); + lock (lockA) { Monitor.Pulse(lockA); } + }) { IsBackground = true }; + t.Start(); + + bool outerResult; + var sw = System.Diagnostics.Stopwatch.StartNew(); + lock (lockA) + { + outerResult = Monitor.Wait(lockA, FailTimeoutMilliseconds); + } + long elapsed = sw.ElapsedMilliseconds; + + Assert.True(outerResult, "Outer Monitor.Wait should have been pulsed"); + Assert.True(elapsed < FailTimeoutMilliseconds / 2, + $"Outer Monitor.Wait took {elapsed}ms — signal was likely stolen by inner wait"); + Assert.False(innerWaitResult, + "Inner Monitor.Wait(lockB) should return false (lockB was never pulsed)"); + + t.Join(FailTimeoutMilliseconds); + }); + } + + private sealed class ReentrantWaitSyncContext : SynchronizationContext + { + private Action? _callback; + + public ReentrantWaitSyncContext(Action callback) + { + _callback = callback; + SetWaitNotificationRequired(); + } + + public override int Wait(IntPtr[] waitHandles, bool waitAll, int millisecondsTimeout) + { + Interlocked.Exchange(ref _callback, null)?.Invoke(); + return base.Wait(waitHandles, waitAll, millisecondsTimeout); + } + } } }