From f3740f09c9ca53e3d990c4ddcb6d41d4aa956449 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Wed, 11 Mar 2026 18:11:08 +0000 Subject: [PATCH 1/4] threading: lock-free fast path for SemaphoreSlim.WaitAsync --- .../src/System/Threading/SemaphoreSlim.cs | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/SemaphoreSlim.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/SemaphoreSlim.cs index a530c22b051d20..df7df059fda8b3 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/SemaphoreSlim.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/SemaphoreSlim.cs @@ -678,6 +678,31 @@ private Task WaitAsyncCore(long millisecondsTimeout, CancellationToken can if (cancellationToken.IsCancellationRequested) return Task.FromCanceled(cancellationToken); + // Fast path: try a lock-free acquire; falls through to the lock if it fails. + // Skipped when m_waitHandle is non-null to keep its state consistent under the lock. + if (m_waitHandle is null) + { + int current = m_currentCount; + // The waiter checks are best-effort: a sync waiter incrementing m_waitCount inside + // the lock may not be visible yet, but the CAS will fail if the count has changed. + if (current > 0 + && Volatile.Read(ref m_asyncHead) is null + && Volatile.Read(ref m_waitCount) == 0 + && Interlocked.CompareExchange(ref m_currentCount, current - 1, current) == current) + { + // Handle the rare race where AvailableWaitHandle was initialised concurrently. + if (current == 1 && m_waitHandle is not null) + { + lock (m_lockObjAndDisposed) + { + if (m_waitHandle is not null && m_currentCount == 0) + m_waitHandle.Reset(); + } + } + return Task.FromResult(true); + } + } + lock (m_lockObjAndDisposed) { // If there are counts available, allow this waiter to succeed. From 7472e160670026e9b7cbadcaec729949cc914d6b Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Wed, 11 Mar 2026 18:54:11 +0000 Subject: [PATCH 2/4] Fix Release() race with WaitAsync CAS fast path Use Interlocked.Add to apply a relative delta to m_currentCount rather than writing back an absolute snapshot-derived value, so concurrent lock-free decrements from the WaitAsync fast path are not overwritten. --- .../src/System/Threading/SemaphoreSlim.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/SemaphoreSlim.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/SemaphoreSlim.cs index df7df059fda8b3..34eaa08b15a62d 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/SemaphoreSlim.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/SemaphoreSlim.cs @@ -924,7 +924,9 @@ public int Release(int releaseCount) waiterTask.TrySetResult(result: true); } } - m_currentCount = currentCount; + // Use Interlocked.Add (relative delta) rather than an absolute write so that + // the lock-free CAS fast path in WaitAsync cannot be overwritten. + Interlocked.Add(ref m_currentCount, currentCount - returnCount); // Exposing wait handle if it is not null if (m_waitHandle is not null && returnCount == 0 && currentCount > 0) From b434b5b3839ba9c950544e00ec1e877d414f7204 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Wed, 11 Mar 2026 19:26:28 +0000 Subject: [PATCH 3/4] Fix double-grant race in WaitAsyncCore slow path Replace plain --m_currentCount with a CAS loop to prevent a double grant when the lock-free WaitAsync fast path decrements m_currentCount between the > 0 check and the decrement in the slow path. WaitCore is safe because m_waitCount++ on lock entry blocks the CAS guard for its entire critical section. WaitAsyncCore has no such protection. --- .../src/System/Threading/SemaphoreSlim.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/SemaphoreSlim.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/SemaphoreSlim.cs index 34eaa08b15a62d..c8e7a699eea888 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/SemaphoreSlim.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/SemaphoreSlim.cs @@ -706,9 +706,13 @@ private Task WaitAsyncCore(long millisecondsTimeout, CancellationToken can lock (m_lockObjAndDisposed) { // If there are counts available, allow this waiter to succeed. - if (m_currentCount > 0) + // Use CAS rather than a plain decrement: the lock-free fast path in WaitAsync + // can decrement m_currentCount concurrently (it holds no lock). + int current = m_currentCount; + while (current > 0 && Interlocked.CompareExchange(ref m_currentCount, current - 1, current) != current) + current = m_currentCount; + if (current > 0) { - --m_currentCount; if (m_waitHandle is not null && m_currentCount == 0) m_waitHandle.Reset(); return Task.FromResult(true); } From fb8aeb09ca90c03331e567c40da91a59f123ef85 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Wed, 11 Mar 2026 19:35:50 +0000 Subject: [PATCH 4/4] Fix double-grant race in WaitCore Apply the same CAS-loop pattern to WaitCore's m_currentCount decrement that was applied to WaitAsyncCore in the previous commit. A fast-path thread that read m_waitCount = 0 before WaitCore's m_waitCount++ can still race with WaitCore's check-at-404 / decrement-at-407 sequence. The CAS loop serializes both operations on m_currentCount atomically. --- .../src/System/Threading/SemaphoreSlim.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/SemaphoreSlim.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/SemaphoreSlim.cs index c8e7a699eea888..68d271ed94a28f 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/SemaphoreSlim.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/SemaphoreSlim.cs @@ -401,10 +401,14 @@ private bool WaitCore(long millisecondsTimeout, CancellationToken cancellationTo // that synchronous waiter succeeds so that they have a chance to release. Debug.Assert(!waitSuccessful || m_currentCount > 0, "If the wait was successful, there should be count available."); - if (m_currentCount > 0) + // Use CAS rather than a plain decrement: the lock-free fast path in WaitAsync + // can decrement m_currentCount concurrently (it holds no lock). + int currentCount = m_currentCount; + while (currentCount > 0 && Interlocked.CompareExchange(ref m_currentCount, currentCount - 1, currentCount) != currentCount) + currentCount = m_currentCount; + if (currentCount > 0) { waitSuccessful = true; - m_currentCount--; } else if (oce is not null) {