12

I've been familiarizing myself with some of the things (that are planned to be) added in C# 8 & .NET Core 3.0, and am unsure on the correct way to implement IAsyncDisposable (at time of writing, this link has literally no guidance).

In particular, it is unclear to me what to do in the case when an instance isn't explicitly disposed - that is, it isn't wrapped in an async using(...) and .DisposeAsync() isn't explicitly called.

My first thought was to do that same thing I'd do when implementing IDisposable:

  • My DisposeAsync() implementation calls a DisposeAsync(bool disposing) with disposing: true
  • Implement a finalizer (with ~MyType()) that calls DisposeAsync(disposing: false)
  • DisposeAsync(bool disposing) actually frees and/or disposes everything, and suppresses finalizing if disposing == true.

My concerns are that there's nothing to await the results of DisposeAsync(bool) in the finalizer, and explicitly waiting in a finalizer seems really really dangerous.

Of course "just leak" also seems less than ideal.

To make this concrete, here's a (simplified) example class that does have a finalizer:

internal sealed class TestAsyncReader: IAsyncDisposable
{
    private bool IsDisposed => Inner == null;
    private TextReader Inner;
    internal TestAsyncReader(TextReader inner)
    {
        Inner = inner;
    }

    // the question is, should this exist?
    ~TestAsyncReader()
    {
        DisposeAsync(disposing: false);
    }

    private ValueTask DisposeAsync(bool disposing)
    {
        // double dispose is legal, but try and do nothing anyway
        if (IsDisposed)
        {
            return default;
        }

        // should a finalizer even exist?
        if (disposing)
        {
            GC.SuppressFinalize(this);
        }

        // in real code some resources explicitly implement IAsyncDisposable,
        //   but for illustration purposes this code has an interface test
        if (Inner is IAsyncDisposable supportsAsync)
        {
            var ret = supportsAsync.DisposeAsync();
            Inner = null;
            return ret;
        }

        // dispose synchronously, which is uninteresting
        Inner.Dispose();
        Inner = null;
        return default;
    }

    public ValueTask DisposeAsync()
    => DisposeAsync(disposing: true);
}

So, is there any guidance around proper handling of leaked IAsyncDisposable instances?

Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
Kevin Montrose
  • 22,191
  • 9
  • 88
  • 137
  • 2
    [Here's the commit](https://github.com/dotnet/corert/pull/6508/commits/395bf09f39f0d9f3233b41b78a52d912cb36b505) that added `IAsyncDisposable` to `Threading.Timer`. Don't know if that helps, but it's an example of its implementation... – Heretic Monkey Apr 14 '19 at 16:25
  • 1
    Dispose does two things: releasing managed resources, and releasing unmanaged resources. I can imagine managed resources needing to be asynchronously released (e.g. a database connection that needs gracefully closing), but I struggle to think of unmanaged resources (memory, file pointers, etc) that need to be released asynchronously. With any managed resource, either it *needs* to be explicitly released (e.g. an event subscription), or ideally it should be released but its finalizer can deal with it if it isn't -- either way, your finalizer shouldn't get involved. – canton7 Apr 14 '19 at 16:29
  • 3
    As an aside, try and avoid writing your own finalizer. It makes your object more expensive to allocate, and it's very easy to mess up (e.g. most people don't know that your finalizable object can be finalized while it's calling into a native method, which causes all sorts of problems). It's much easier and safer to use [`SafeHandle`](https://learn.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.safehandle?view=netframework-4.7.2) and its derived classes. That sidesteps your problem. – canton7 Apr 14 '19 at 16:32
  • 3
    `Dispose(bool disposing)` was a horrible pattern. For the love of all that is holy, let's not do the same thing with `DisposeAsync`. – Stephen Cleary Apr 17 '19 at 13:01
  • @StephenCleary How do we integrate finalizers and DisposeAsync? – bboyle1234 Sep 27 '19 at 13:31
  • 3
    @bboyle1234: I [have always recommended](https://blog.stephencleary.com/2009/08/how-to-implement-idisposable-and.html) only using finalizers for types that wrap an unmanaged resource. Those types should only have a synchronous `Dispose`. If you follow that pattern, there will never be a need to integrate finalizers with `DisposeAsync`. – Stephen Cleary Sep 27 '19 at 13:58

2 Answers2

12

Basing on examples of how it's implemented inside .NET Core classes (like here) and some recommendations from there, I'd say that when you need to implement IAsyncDisposable, the good practice would be to implement both IAsyncDisposable and IDisposable. In this case IAsyncDisposable will be only responsible for explicit scenarios when asyncronus disposal is needed, while IDisposable is supposed to be implemented as usual according to disposable pattern practices and it's going to serve all fallback scenarios including those when things come to finalization. Thus you don't need to have anything like DisposeAsync(bool disposing) - the asynchronous disposal cannot and shouldn't happen in a finalizer. The only bad news that you'll have to support both paths for resources reclaiming (synchronous and asynchronous).

Dmytro Mukalov
  • 1,949
  • 1
  • 9
  • 14
0

Microsoft released their own guidance regarding this problem.
As in the accepted answer you should implement both interfaces

If you implement the IAsyncDisposable interface but not the IDisposable interface, your app can potentially leak resources. If a class implements IAsyncDisposable, but not IDisposable, and a consumer only calls Dispose, your implementation would never call DisposeAsync. This would result in a resource leak.

But also you may need to implement 2 dispose patterns:


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

class ExampleConjunctiveDisposableusing : 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().ConfigureAwait(false);

        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();
        }

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

        _asyncDisposableResource = null;
        _disposableResource = null;
    }
}
Mr Patience
  • 1,564
  • 16
  • 30