Wrapping SemaphoreSlim.WaitAsync with an IDisposable

Question

The usual way of dealing with a SemaphoreSlim is

await semaphore.WaitAsync();
try
{
 await DoStuff();
}
finally
{
 semaphore.Release();
}

To help with not making mistakes like not releasing the semaphore or putting the semaphore in the try block, I created this extension class

public static class SemaphoreSlimExtensions
{
 public static async ValueTask<SemaphoreSlimReleaser> WaitDisposableAsync(this SemaphoreSlim semaphoreSlim)
 {
 await semaphoreSlim.WaitAsync();
 return new SemaphoreSlimReleaser(semaphoreSlim);
 }
 public sealed class SemaphoreSlimReleaser : IDisposable
 {
 private readonly SemaphoreSlim _semaphoreSlim;
 private bool _isDisposed = false;
 public SemaphoreSlimReleaser(SemaphoreSlim semaphoreSlim)
 {
 this._semaphoreSlim = semaphoreSlim;
 }
 public void Dispose()
 {
 if (_isDisposed)
 {
 return;
 }
 DisposeInternal();
 GC.SuppressFinalize(this);
 }
 private void DisposeInternal()
 {
 if (_isDisposed)
 {
 return;
 }
 _semaphoreSlim.Release();
 _isDisposed = true;
 }
 ~SemaphoreSlimReleaser()
 {
 DisposeInternal();
 }
 }
}

This can then be used like

public async Task DoProtected()
{
 using var @lock = await semaphoreSlim.WaitDisposableAsync();
 await DoStuff();
}

Do you see any problem with this wrapping? Or is there maybe a package that already has this wrapping?

I also ran a simple benchmark with this result

BenchmarkDotNet=v0.13.5, OS=Windows 11 (10.0.22621.2861/22H2/2022Update/SunValley2)
Intel Core i7-8565U CPU 1.80GHz (Whiskey Lake), 1 CPU, 8 logical and 4 physical cores
.NET SDK=8.0.100
 [Host] : .NET 6.0.25 (6.0.2523.51912), X64 RyuJIT AVX2
 DefaultJob : .NET 6.0.25 (6.0.2523.51912), X64 RyuJIT AVX2
| Method | Mean | Error | StdDev | Ratio | RatioSD | Allocated | Alloc Ratio |
|------- |---------:|---------:|---------:|------:|--------:|----------:|------------:|
| Old | 15.73 ms | 0.223 ms | 0.197 ms | 1.00 | 0.00 | 397 B | 1.00 |
| New | 15.64 ms | 0.210 ms | 0.206 ms | 0.99 | 0.02 | 496 B | 1.25 |

The benchmark code is

[MemoryDiagnoser]
[SimpleJob]
public class SemaphoreExtensionBenchmark
{
 private readonly SemaphoreSlim _semaphoreSlim = new(1, 1);
 [Benchmark(Baseline = true)]
 public async Task Old()
 {
 await _semaphoreSlim.WaitAsync();
 try
 {
 await Task.Delay(10);
 }
 finally
 {
 _semaphoreSlim.Release();
 }
 }
 [Benchmark]
 public async Task New()
 {
 using var releaser = await _semaphoreSlim.WaitDisposableAsync();
 await Task.Delay(10);
 }
}

• Peter Csala: Please do not amend the code of your question after there are answers. (The posts become out of sync with a question). If you want to have a second round review please feel free to post a new question and refer to this.
• Jesse C. Slicer: There are a handful of overloads to WaitAsync() - you might want to emulate each of them in extension methods.

Answer 1

Disclaimer: This is not a review just a bit lengthier comment.

I think you have just shifted the problem. One might forget to use using block for the SemaphoreSlimReleaser and the release of the semaphore is in God's hands...

Another note that you have implemented the IDisposable interface in a naive way, please check the related documentation or the code analysis related rule violation to see how to implement it properly.

• Ackdari: Thank you for pointing that out, I changed it and also changed he struct to be a class.
• Ackdari: Regarding of the shifting of the problem: Yes, you are right, however there are analyzer to warn you if you didn't dispose an IDisposable, but I never saw something similar for locking mechanisms
• Peter Csala: @Ackdari SonarQube has somewhat similar rule: rules.sonarsource.com/csharp/tag/multi-threading/RSPEC-2222

Answer 2

Should follow the dispose pattern as .net IDispose

If an object's Dispose method is called more than once, the object must ignore all calls after the first one

The current implmenetation if dispose is called multiple times will continue to release and I don't think that would an expected behavior.

The other thing to think about is when doing using, without the braces, it is released at the end of the scope. Nothing you can control but something to be aware of that extra statements could be part of the locking that was unintended and typically you want locks to be locked as short of time as possible.

• Ackdari: Thanks I changed the IDisposable implementation.