Is it safe to dispose a SemaphoreSlim while waiting for pending operations to cancel?

Question

I've had to use SemaphoreSlim to ensure single-threaded access to some parts of my code, and would like to make sure that I'm disposing of everything correctly. Suppose I have the following class:

public class Foo
{
    private readonly CancellationTokenSource _canceller = new CancellationTokenSource();
    private readonly SemaphoreSlim _semaphore = new SemaphoreSlim(1);

    ~Foo()
    {
        Dispose(false);
        GC.SuppressFinalize(this);
    }

    public void Dispose()
    {
        Dispose(true);
    }

    protected void Dispose(bool disposing)
    {
        if (_disposed)
            return;

        _canceller.Cancel();

        if (_disposing)
            _semaphore.Dispose();

        _disposed = true;
    }

    public async Task ExecuteAsync()
    {
        try
        {
            await _semaphore.WaitAsync(_canceller.Token);
        }
        catch (OperationCanceledException)
        {
            throw new ObjectDisposedException(nameof(Foo), "Cannot execute on a Foo after it has been disposed");
        }
        
        try
        {
            // Critical section
        }
        finally
        {
            _semaphore.Release();
        }
    }
}

When I call Dispose(true), I'm effectively executing the following lines one after the other:

_canceller.Cancel();
_semaphore.Dispose();

My question is, what happens to any other threads/tasks that are currently waiting for the critical section? Can I guarantee that they will always see the cancellation first, and so will not have a problem with the semaphore being disposed? This hasn't caused any issues so far, but that doesn't mean that it is safe.

Answer 1

The documentation for SemaphoreSlim seem very specific about your question.

All public and protected members of SemaphoreSlim are thread-safe and may be used concurrently from multiple threads, with the exception of Dispose(), which must be used only when all other operations on the SemaphoreSlim have completed.

The only real way to Guarantee that all other operations have completed is to make sure all the tasks have been completed as the example in the documentation shows.

public class Foo
{
    private readonly List<Task> _semaphoreTasks = new list<Task>();

    protected void Dispose(bool disposing)
    {
        if (_disposed)
            return;

        _canceller.Cancel();

        if (_disposing) 
        {
            Task.WaitAll(tasks);
            _semaphore.Dispose();
        }

        _disposed = true;
    }

    public async Task ExecuteAsync()
    {
        try
        {
            var task = _semaphore.WaitAsync(_canceller.Token);
            _semaphoreTasks.Add(task);
            await task;
        // ...

This would also most likely require making sure things are added to the list while the dispose is executing. (You could set the private class variable to a local method variable and then set the class variable to null)