[API Proposal]: SemaphoreSlim.EnterScope

[EgorBo]

Background and motivation

A classic pattern for locks in async code looks like this:

readonly SemaphoreSlim _semaphoreSlim = new (1);

async Task DoWork()
{
    await _semaphoreSlim.WaitAsync();
    try
    {
        // some work
    }
    finally
    {
        _semaphoreSlim.Release();
    }
}

This pattern feels a bit too verbose compared to non-async lock and it's important to keep in mind that Release has to be called in finally block to avoid dead-locks. I propose we add a new API for SemaphoreSlim so the same program can be simplified down to

async Task DoWork1()
{
    using (var scope = await _semaphoreSlim.EnterScope())
    {
        // do some work
    }
}

// or

async Task DoWork2()
{
    using var _ = await _semaphoreSlim.EnterScope();
    // do some work
}

Similarily to Lock.EnterScope API.

API Proposal

namespace System.Threading;

public class SemaphoreSlim
{
+   public ValueTask<SemaphoreSlim.Scope> EnterScope();
+   public ValueTask<SemaphoreSlim.Scope> EnterScope(CancellationToken cancellationToken);
+   public ValueTask<SemaphoreSlim.Scope> EnterScope(int millisecondsTimeout);
+   public ValueTask<SemaphoreSlim.Scope> EnterScope(TimeSpan timeout, CancellationToken cancellationToken);
+   public ValueTask<SemaphoreSlim.Scope> EnterScope(int millisecondsTimeout, CancellationToken cancellationToken);

+   public readonly struct Scope : IDisposable
+   {
+       public void Dispose();
+   }
}

I am not sure about the overloads, I just copied the exising ones from WaitAsync for consistency, I'd pesonally add only these:

+   public ValueTask<SemaphoreSlim.Scope> EnterScope(CancellationToken cancellationToken = default);
+   public ValueTask<SemaphoreSlim.Scope> EnterScope(TimeSpan timeout, CancellationToken cancellationToken = default);

Also, should it be EnterScopeAsync ?

API Usage

using var _ = await _semaphoreSlim.EnterScope();

Sadly, C# doesn't allow to omit the variable so we could have something like:

using await _semaphoreSlim.EnterScope();
// or at least:
using _ = await _semaphoreSlim.EnterScope();

Alternative Designs

We can extend the lock keyword in C# to support async/await like we recently extended it to support the new Lock type. I think it's a popular pain point when developers (especially newcomers) try to use lock (obj) for async code and then have to google how to workaround the compilation failure.

readonly SemaphoreSlim _semaphoreSlim = new (1);

async Task DoWork()
{
    lock (_semaphoreSlim)
    // await lock (_semaphoreSlim) ?
    {
    }
}

Or at very least, Roslyn could suggest to use SemaphoreSlim in the error text when user tries to use normal lock for async blocks.

Risks

No response

[dotnet-policy-service[bot]]

Tagging subscribers to this area: @mangod9 See info in area-owners.md if you want to be subscribed.

[jcotton42]

In the case of

+public sealed class SemaphoreSlimReleaser(SemaphoreSlim semaphore) : IDisposable
+{
+   public void Dispose() => semaphore.Release();
+}

Would we maybe want an idDisposed bool field or similar, to prevent double release by disposing the same releaser twice?

[MihaZupan]

I'd make EnterScope a ValueTask and change SemaphoreSlimReleaser to a public struct Scope nested type (similar to Lock's scope in #34812).

[y0ung3r]

Also, should it be EnterScopeAsync?

My personal opinion - yes

[aloraman]

This pattern of issuing a small cookie-object, while changing the state of the owner, and reverting the owner's state on cookie disposal - it's quite familiar and somewhat well-known. I'm so accustomed to it, that I would've chosen the name SemaphoreReleaseCookie without a hint of hesitation. That said, MSFT uses the term 'cookie' mostly for stack canary scenarios, so it doesn't fit here. Nevertheless, I would suggest the following:

• Do make this cookie object not a class, but a readonly struct.
• Don't bother with double-release prevention, IMHO, additional security for advanced usage scenarios just isn't worth the overhead.

[JakeYallop]

I've used this pattern so many times, I'm surprised something similar hasn't been proposed before. I think the name should be EnterScopeAsync, to fit with basically every other API that returns a Task or ValueTask.

[Rob-Hague]

I have also used this pattern a lot and would welcome something more idiomatic. One thing though, is that in practically all cases I am also using the equivalent sync pattern. It would be nice if both async and sync variants of the pattern were available.

By the way, this exists already in Stephen Cleary's package (https://github.com/StephenCleary/AsyncEx/blob/master/src/Nito.AsyncEx.Tasks/SemaphoreSlimExtensions.cs). If we were going there, it would be even nicer if there were a dedicated type which can participate in synchronous and asynchronous mutual exclusion, including with Monitor.Wait&Pulse-like semantics. Basically, something similar to Cleary's AsyncMonitor/AsyncLock (https://github.com/StephenCleary/AsyncEx/blob/master/src/Nito.AsyncEx.Coordination/AsyncMonitor.cs) but perhaps that's a separate discussion.

[lilinus]

I'd make EnterScope a ValueTask and change SemaphoreSlimReleaser to a public struct Scope nested type (similar to Lock's scope in #34812).

Just to point out a potential caveat: If the scope has a field that prevents double-releasing and is a struct, would it not still be possible to release twice if the struct is copied?

[jaredpar]

Similarily to Lock.EnterScope API.

Given the similarity should we consider expanding the new C# lock support to include this type? When we started the work on Lock.EnterScope in was specific to that type because there were no other examples.

[EgorBo]

Similarily to Lock.EnterScope API.

Given the similarity should we consider expanding the new C# lock support to include this type? When we started the work on Lock.EnterScope in was specific to that type because there were no other examples.

I guess the main problem that it's not obvious what it should do, the wait in this case is async, so should it look like this:

lock (obj) // await is implicit, requires method to be async. obj must be SemaphoreSlim (or some duck-typing check)
{
}

or it needs to be a new construct:

await lock (obj)
{
}

[quinmars]

I think it should be await lock (...) analog to using and foreach. More problematic is how you can pass the cancellation token and the timeout and how you can .ConfigureAwait?