Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ReentrantSemaphore - Implement EnterAsync with timeout #1362

Open
robert-at-pieces opened this issue Dec 19, 2024 · 3 comments
Open

ReentrantSemaphore - Implement EnterAsync with timeout #1362

robert-at-pieces opened this issue Dec 19, 2024 · 3 comments

Comments

@robert-at-pieces
Copy link

Is your feature request related to a problem? Please describe.

SemaphoreSlim and AsyncSemaphore have been banned via analyzers with the replacement preference being ReentrantSemaphore due to it being JTF aware.

AsyncSemaphore's EnterAsync implementation allows for a timeout parameter

ReentrantSemaphore's ExecuteAsync implementation always enters the underlying AsyncSemphare with an infinite timeout

Describe the solution you'd like

AsyncSemaphore supports an EnterAsync timeout. I would like to see ReentrantSemaphore allow for this parameter to be passed as an argument during ExecuteAsync.

Describe alternatives you've considered

Unless I am mistaken,
You can partially work around this by passing a cancellation token that times out, but this timeout is not specific to acquiring the lock. If the critical section is a long running operation, it is possible that a passed in cancellation token would cancel even though entrance into the critical section was achieved.

@weltkante
Copy link
Contributor

weltkante commented Dec 19, 2024

it is possible that a passed in cancellation token would cancel even though entrance into the critical section was achieved

but then the call would have succeed and not thrown, so the caller would have first seen that it entered and acquired the lock, construct whatever scope is necessary to safely release it when leaving the scope, and only then (by its own choice) may or may not check the cancellation token again

any other implementation would be bugged regardless of your particular usecase via a timeout and it would never be safe to pass a cancellation token

so while having a particular API with timeout support probably might be more efficient it shouldn't be necessary for correctness

@robert-at-pieces
Copy link
Author

@weltkante

Thank you for the response. Perhaps I did misunderstand the 'ExecuteAsync' implementation.

Would it be correct to say that once the the operation callback is invoked OperationCancelled exception will not be thrown and it is the caller's responsibility to check the CancellationToken inside of the Task that was passed as a parameter?

@weltkante
Copy link
Contributor

yes, all checks in the code happen before, and if the operation itself wants to check it then it'll do it on its own terms

(also I'm not part of the repo team, just subscribed to the issue list, so feel free to wait for an 'official' confirmation)

@AArnott AArnott changed the title ReentrantSemaphore - Implement EntryAsync with timeout ReentrantSemaphore - Implement EnterAsync with timeout Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants