IDisposable questions about specific case

April 28, 2022

Question #1:

StartAsync handles the disposal of _clientWebSocket and _tokenSource. So do I really need to dispose these in Dispose() as well? I think I should keep _semaphore.Dispose() only in the Dispose(), because my code already handles the rest.

Question #2:

What if the user forgets to call Dispose() or wrap it in using? It’s usually solved by calling Dispose() in the deconstructor/finalizer but in this case my class is sealed.

public sealed class Client : IDisposable
{
    private readonly SemaphoreSlim _semaphore = new(1, 1);

    private ClientWebSocket? _clientWebSocket;
    private CancellationTokenSource? _tokenSource;

    public void Dispose()
    {
        _semaphore.Dispose();

        // TODO: Do I need to dispose these since my code below does that? 
        _clientWebSocket?.Dispose();
        _clientWebSocket = null;

        _tokenSource?.Cancel();
        _tokenSource = null;
    }

    public Task StartAsync()
    {
        _clientWebSocket = new ClientWebSocket();
        _tokenSource = new CancellationTokenSource();

        try
        {
            ...
        }
        catch (Exception ex)
        {
        }
        finally
        {
            _clientWebSocket?.Dispose();
            _clientWebSocket = null;

            _tokenSource?.Cancel();
            _tokenSource = null;
        }
    }

    public ValueTask SendAsync()
    {
        if (_clientWebSocket is { State: WebSocketState.Open })
        {
            return;
        }

        ...
    }
}

>Solution :

StartAsync handles the disposal of _clientWebSocket and _tokenSource. So do I really need to dispose these in Dispose() as well? I think I should keep _semaphore.Dispose() only in the Dispose(), because my code already handles the disposal of the rest.

Then why are they fields? It sounds like they should be local variables, in which case they wouldn’t be part of your class’ dispose method either way.

Also forget the pattern you’re using and use using instead.

What if the user forgets to call Dispose() or wrap it in using?

It’s their choice, but there is a Roslyn analyzer warning for that: CA2000

It’s usually solved by calling Dispose() in the deconstructor/finalizer

Less usually, more always, but you are correct. You want your finalizer to clean up after your object if the caller doesn’t, with the obvious caveat of threading (what thread will the finalizer run on? The answer to that question prevents you from disposing of OpenGL handles in finalizers for example).