SemaphoreSlim extension method for safely handling cancellation and disposal
Review



Is there a better way to safely manage the cancellation and disposal of SemaphoreSlim instances?

You've made a pattern to safely manage completing / aborting a task that is acquiring or has acquired a lock on a semaphore, not to safely manage the cancellation and disposal of SemaphoreSlim instances.

You should only release the semaphore if you are sure you have acquired a lock. So take await semaphore.WaitAsync(cancellationToken); out the try block (as suggested in the comments).

You should also provide overloads or equivalent checks that return Task&lt;T&gt; and/or accept a CancellationToken as argument for the task.

In your exception handlers you might get false negatives, in that it's possible an OperationCanceledException or ObjectDisposedException was raised while condition cancellationToken.IsCancellationRequested was met, but coming from a different source than the cancellationToken. I don't know whether this is a big issue, since cancellation was requested anyway. But you should definately document in which scenarios exceptions could be propagated to the consumer.

Rather than the catch - if - throw blocks, you could use catch when (condition) blocks. For instance, catch (OperationCanceledException ex) when (cancellationToken == ex.CancellationToken). This is more compact, doesn't require you rethrow yourself and allows you to focus on silently capturing these exceptions on certain conditions. You might want to log something on such errors.

I'm not convinced this extension method should take the semaphore as source object. To me, the semaphore is more of a utility used to execute these tasks. Maybe a static Helper class would have been the better choice.