Adam V. Steele
Adam V. Steele

Reputation: 639

IAsyncDisposable reference implementation error?

Microsoft provides a refence implementation for classes that need to implement both IDisposable and IAsyncDisposable, say because they have members of both of these.

https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/implementing-disposeasync

I include the full implementation below. These lines in Dispose(disposing) are the part I don't understand.

(_asyncDisposableResource as IDisposable)?.Dispose();
...
_asyncDisposableResource = null;

It seems like if I have an instance of CustomDisposable and Dispose() gets called on it before DisposeAsync() then the _asyncDispoableResource field will have Dispose() called on it instead of DisposeAsync (if it has one), and then get set to null unconditionally. Seems like _asyncDispoableResource never gets disposed of properly in this case, even if DisposeAsync() gets called on the CustomDisposable instance later.

Full reference code:

using System;
using System.IO;
using System.Threading.Tasks;

namespace Samples
{
    public class CustomDisposable : IDisposable, IAsyncDisposable
    {
        IDisposable _disposableResource = new MemoryStream();
        IAsyncDisposable _asyncDisposableResource = new MemoryStream();

        public void Dispose()
        {
            Dispose(disposing: true);
            GC.SuppressFinalize(this);
        }

        public async ValueTask DisposeAsync()
        {
            await DisposeAsyncCore();

            Dispose(disposing: false);
            GC.SuppressFinalize(this);
        }

        protected virtual void Dispose(bool disposing)
        {
            if (disposing)
            {
                _disposableResource?.Dispose();
                (_asyncDisposableResource as IDisposable)?.Dispose();
            }

            _disposableResource = null;
            _asyncDisposableResource = null;
        }

        protected virtual async ValueTask DisposeAsyncCore()
        {
            if (_asyncDisposableResource is not null)
            {
                await _asyncDisposableResource.DisposeAsync().ConfigureAwait(false);
            }

            if (_disposableResource is IAsyncDisposable disposable)
            {
                await disposable.DisposeAsync().ConfigureAwait(false);
            }
            else
            {
                _disposableResource.Dispose();
            }

            _asyncDisposableResource = null;
            _disposableResource = null;
        }
    }
}

Upvotes: 3

Views: 1154

Answers (1)

Alex
Alex

Reputation: 18546

Quoting your reference:

It is typical when implementing the IAsyncDisposable interface that classes will also implement the IDisposable interface. A good implementation pattern of the IAsyncDisposable interface is to be prepared for either synchronous or asynchronous dispose.

This means that it should be sufficient to dispose an object via "classic" Dispose, it should not matter that DisposeAsync is never called. This is why the resource is set to null, so it is clear that it has already been disposed asynchonously. The actual implementation of the disposable reference has to be prepared for this.

Upvotes: 2

Related Questions