0

The class below is not sealed, which means it is considered inheritable. I took IDisposable/IAsyncDisposable implementation from here and I'm trying to understand why the .Dispose calls are duplicated in both Dispose and DisposeAsync. Will someone be able to explain to me how it works more deeply/or I would say low level, so I actually know how to use it in future?

public class Client : IDisposable, IAsyncDisposable
{
    private readonly ILogger<Client> _logger;
    private readonly Channel<string> _outputChannel;
    private ClientWebSocket? _clientWebSocket;
    private CancellationTokenSource? _cancellationSource;
    private Task _receiving = Task.CompletedTask;
    private Task _sending = Task.CompletedTask;

    public Client(ILoggerFactory? loggerFactory = default)
    {
        _logger = (loggerFactory ?? NullLoggerFactory.Instance).CreateLogger<Client>();
    }

    public bool IsDisposed { get; protected set; }

    ...

    /// <summary>
    ///     Checks if this object has been disposed.
    /// </summary>
    /// <exception cref="ObjectDisposedException">Thrown if the object has been disposed.</exception>
    [MethodImpl(MethodImplOptions.AggressiveInlining)]
    protected void DoDisposeChecks()
    {
        if (IsDisposed)
        {
            throw new ObjectDisposedException(nameof(Client));
        }
    }

    /// <summary>
    ///     Disposes of managed and unmanaged resources.
    /// </summary>
    /// <param name="disposeManaged">A value indicating whether or not to dispose of managed resources.</param>
    protected virtual void Dispose(bool disposeManaged)
    {
        if (IsDisposed)
        {
            return;
        }

        if (disposeManaged)
        {
            _logger.LogDebug("Socket {Id} is disposing", Id);

            _clientWebSocket?.Dispose();
            _cancellationSource?.Dispose();

            _outputChannel.Writer.TryComplete();

            _logger.LogDebug("Socket {Id} disposed", Id);
        }

        IsDisposed = true;
    }

    /// <summary>
    ///     Asynchronously disposes of managed resources.
    /// </summary>
    /// <returns>A task representing the asynchronous operation.</returns>
    protected virtual ValueTask DisposeAsyncCore()
    {
        if (IsDisposed)
        {
            return default;
        }

        _logger.LogDebug("Socket {Id} is disposing", Id);

        _clientWebSocket?.Dispose();
        _cancellationSource?.Dispose();

        _outputChannel.Writer.TryComplete();

        _logger.LogDebug("Socket {Id} disposed", Id);

        IsDisposed = true;

        return default;
    }

    /// <inheritdoc />
    public async ValueTask DisposeAsync()
    {
        await DisposeAsyncCore().ConfigureAwait(false);
        Dispose(false);
        GC.SuppressFinalize(this);
    }

    /// <inheritdoc />
    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }
}
nop
  • 4,711
  • 6
  • 32
  • 93
  • 1
    That class doesn't have a finalizer so shouldn't be calling GC.SuppressFinalize at all. – Liam Apr 25 '22 at 09:28
  • @Liam, https://github.com/carlst99/DbgCensus/blob/cc19d4282e774826e98ca4a3287efea1ea0152a0/DbgCensus.EventStream/BaseEventStreamClient.cs doesn't seem to have it too (the place I took this implementation from). – nop Apr 25 '22 at 09:30
  • 1
    I mean I might hit myself in the head with a hammer, doesn't mean you should copy me :D – Liam Apr 25 '22 at 09:32
  • @Liam, true. https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/implementing-disposeasync#implement-the-async-dispose-pattern Microsoft's implementation doesn't have the duplicated calls in both .Dispose and .DisposeAsync – nop Apr 25 '22 at 09:33
  • Someone seems to have very much over thought this. Seems to me this should have a simple `IDisposable` with a Dispose call. This is a common anti pattern you see around. IMO it's premature optimisation. – Liam Apr 25 '22 at 09:35
  • the docs specifically said, *"If obj does not have a finalizer or the GC has already signaled the finalizer thread to run the finalizer, the call to the SuppressFinalize method has no effect"* in regard to `GC.SuppressFinalize`. as for the original question, `IDisposable` called on synchronous run while `IAsyncDisposable` called on asyc only, [this been discussed before](https://stackoverflow.com/questions/55677445/whats-the-recommended-way-to-deal-with-leaked-iasyncdisposable-instances). – Bagus Tesa Apr 25 '22 at 09:38

0 Answers0