Extend allowed SemaphoreSlim wait TimeSpan values to match Timer and other APIs #117398
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
the
needs-area-label
eduardo-vp
added
area-System.Threading
and removed
needs-area-label
|
Tagging subscribers to this area: @mangod9 |
eduardo-vp
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR extends SemaphoreSlim to support TimeSpan timeouts up to Timer.MaxSupportedTimeout (instead of int.MaxValue), refactors internal timeout logic to use unsigned timeouts, updates error messages, and adjusts tests accordingly.
- Updated
SemaphoreSlimwait methods (sync and async) to useuinttimeouts and validate againstTimer.MaxSupportedTimeout. - Added a
uintoverload inTimeoutHelper.UpdateTimeOutfor unsigned infinite and large-timeout support. - Adjusted tests and resource strings to reflect the new maximum timeout and renamed the error message resource.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/libraries/System.Threading/tests/SemaphoreSlimTests.cs | Extended tests to cover the new max TimeSpan values and negative cases |
| src/libraries/System.Private.CoreLib/src/System/Threading/TimeoutHelper.cs | Introduced uint overload for UpdateTimeOut to support unsigned infinite timeouts |
| src/libraries/System.Private.CoreLib/src/System/Threading/SemaphoreSlim.cs | Refactored Wait/WaitAsync to use uint timeouts, use Timer.MaxSupportedTimeout, and updated exception messages |
| src/libraries/System.Private.CoreLib/src/Resources/Strings.resx | Added SemaphoreSlim_Wait_TimeSpanTimeoutWrong resource for the updated validation |
Comments suppressed due to low confidence (1)
src/libraries/System.Private.CoreLib/src/System/Threading/SemaphoreSlim.cs:265
- The code refers to
timeout.Infinite, which does not exist. It should comparetotalMillisecondsagainstTimeout.Infiniteto detect the infinite-timeout case.
return Wait(totalMilliseconds == timeout.Infinite ? Timeout.UnsignedInfinite : (uint)totalMilliseconds, cancellationToken);
ba94339 to
53ec2ab
Compare
This was referenced Jul 8, 2025
Open
eduardo-vp
force-pushed
the
extend-semaphore-wait
branch
from
63df413 to
d2a4063
Compare
jkotas
reviewed
Jul 15, 2025
src/libraries/System.Private.CoreLib/src/Resources/Strings.resx
Outdated
Show resolved
Hide resolved
jkotas
reviewed
Jul 15, 2025
src/libraries/System.Private.CoreLib/src/System/Threading/TimeoutHelper.cs
Outdated
Show resolved
Hide resolved
This was referenced Jul 16, 2025
jkotas
reviewed
Jul 22, 2025
src/libraries/System.Private.CoreLib/src/Resources/Strings.resx
Outdated
Show resolved
Hide resolved
jkotas
reviewed
Jul 22, 2025
src/libraries/System.Private.CoreLib/src/System/Threading/TimeoutHelper.cs
Outdated
Show resolved
Hide resolved
jkotas
reviewed
Jul 22, 2025
src/libraries/System.Private.CoreLib/src/System/Threading/SemaphoreSlim.cs
Outdated
Show resolved
Hide resolved
jkotas
reviewed
Jul 22, 2025
src/libraries/System.Private.CoreLib/src/System/Threading/SemaphoreSlim.cs
Outdated
Show resolved
Hide resolved
jkotas
reviewed
Jul 22, 2025
src/libraries/System.Private.CoreLib/src/System/Threading/SemaphoreSlim.cs
Outdated
Show resolved
Hide resolved
jkotas
reviewed
Jul 22, 2025
src/libraries/System.Private.CoreLib/src/System/Threading/SemaphoreSlim.cs
Outdated
Show resolved
Hide resolved
eduardo-vp
force-pushed
the
extend-semaphore-wait
branch
from
f2d735e to
20157b5
Compare
jkotas
reviewed
Jul 31, 2025
src/libraries/System.Private.CoreLib/src/Resources/Strings.resx
Outdated
Show resolved
Hide resolved
jkotas
reviewed
Jul 31, 2025
src/libraries/System.Private.CoreLib/src/System/Threading/ManualResetEventSlim.cs
Outdated
Show resolved
Hide resolved
This was referenced Jul 31, 2025
eduardo-vp
force-pushed
the
extend-semaphore-wait
branch
from
40048e1 to
321730c
Compare
jkotas
reviewed
Aug 12, 2025
| @@ -211,8 +211,7 @@ public void Wait(CancellationToken cancellationToken) | |||
| /// <returns>true if the current thread successfully entered the <see cref="SemaphoreSlim"/>; | |||
| /// otherwise, false.</returns> | |||
| /// <exception cref="ArgumentOutOfRangeException"><paramref name="timeout"/> is a negative | |||
| /// number other than -1 milliseconds, which represents an infinite time-out -or- timeout is greater | |||
There was a problem hiding this comment.
The doc comments are not the official sources for the docs. Could you please submit PR to https://github.com/dotnet/dotnet-api-docs/ to add a note that the ArgumentOutOfRangeException is no longer thrown for timeout greater than int.MaxValue in .NET 10?
There was a problem hiding this comment.
PR here dotnet/dotnet-api-docs#11677
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #116877.
Allow SemaphoreSlim to accept a TimeSpan that goes all the way until UInt32.MaxValue - 1 milliseconds, matching the range that other APIs support.