C#•15mo ago

✅ is it a good disposable class?

public class AsyncDisposable                                      
    : Disposable,                                                 
    IAsyncDisposable                                              
{                                                                 
    public bool IsDisposedAsync { get; private set; }             
                                                                  
    public async ValueTask DisposeAsync()                         
    {                                                             
        if ( IsDisposedAsync || IsDisposed )                      
            return;                                               
                                                                  
        _disposingСancellationTokenSource.Cancel();               
                                                                  
        await OnDisposeAsync.InvokeAsync();                       
                                                                  
        await DisposeAsyncCore()                                   
            .ConfigureAwait( continueOnCapturedContext: false );   
                                                                  
        IsDisposedAsync = true;                                   
                                                                  
        Dispose();                                                
        GC.SuppressFinalize( this );                              
    }                                                             
                                                                  
    protected virtual ValueTask DisposeAsyncCore() =>             
        default;                                                  
                                                                  
    public Func<ValueTask> OnDisposeAsync { get; set; }           
}

public class AsyncDisposable                                      
    : Disposable,                                                 
    IAsyncDisposable                                              
{                                                                 
    public bool IsDisposedAsync { get; private set; }             
                                                                  
    public async ValueTask DisposeAsync()                         
    {                                                             
        if ( IsDisposedAsync || IsDisposed )                      
            return;                                               
                                                                  
        _disposingСancellationTokenSource.Cancel();               
                                                                  
        await OnDisposeAsync.InvokeAsync();                       
                                                                  
        await DisposeAsyncCore()                                   
            .ConfigureAwait( continueOnCapturedContext: false );   
                                                                  
        IsDisposedAsync = true;                                   
                                                                  
        Dispose();                                                
        GC.SuppressFinalize( this );                              
    }                                                             
                                                                  
    protected virtual ValueTask DisposeAsyncCore() =>             
        default;                                                  
                                                                  
    public Func<ValueTask> OnDisposeAsync { get; set; }           
}

what can I change?

37 Replies

mtreit•15mo ago

Why would you ever need an async dispose?

SWEETPONY•15mo ago

because I want asynchronous disposal operation

mtreit•15mo ago

Why? Dispose is for releasing unmanaged resources which to my knowledge almost never requires I/O...so why would async be useful?

JakenVeina•15mo ago

the designers thought it was useful enough to add it to .NET Core 3

JakenVeina•15mo ago

https://learn.microsoft.com/en-us/dotnet/api/system.iasyncdisposable?view=net-7.0

IAsyncDisposable Interface (System)

Provides a mechanism for releasing unmanaged resources asynchronously.

JakenVeina•15mo ago

logging strikes me as a good scenario in fact, I think that's the one time I literally implemented that interface for a Microsoft.Extensions.Logging implementation

SWEETPONY•15mo ago

when working with a resource that may be shared across multiple threads (such as a database connection), deadlocks can occur if multiple threads attempt to access the resource at the same time. Asynchronous operations can help prevent deadlocks by allowing multiple threads to continue executing while waiting for the resource to become available

JakenVeina•15mo ago

on dispose of the ILoggerProvider I want to cleanly make sure and dump all logs I haven't yet sent to the database

mtreit•15mo ago

I can envision such a scenario but I am skeptical most programmers would ever need it

JakenVeina•15mo ago

for sure, it's rare the way most people are going to encounter it is via ServiceProvider from Microsoft.Extensions.DependencyInjection

mtreit•15mo ago

I question whoever wrote that. async does not help prevent deadlocks

JakenVeina•15mo ago

it's a nicer way to handle thread signaling, arguably, than locking or semaphores or whatever, but yeah async code doesn't prevent deadlocks any more than locking or signaling does

mtreit•15mo ago

async is useful for not exhausting the thread pool. That's pretty much it.

SWEETPONY•15mo ago

I wrote it for asynchronous operations as I said

JakenVeina•15mo ago

anyway, to the original question, I would question the need for an AsyncDisposable base class, most of the time

mtreit•15mo ago

But why?

JakenVeina•15mo ago

and I definitely don't buy the public OnDisposeAsync property

mtreit•15mo ago

I admit I don't do any UI programming and I guess async is useful there for not blocking the UI thread

JakenVeina•15mo ago

and there's no parameter on DisposeAsyncCore() indicating whether this is managed or unmanaged disposale and there's no finalizer defined, so GC.SuppressFinalize(this) is worthless and IsDisposedAsync is kinda nonsense

SWEETPONY•15mo ago

for smth like this:

public class CustomAsyncDataProvider : IAsyncEnumerable<int>
{
    public IAsyncEnumerator<int> GetAsyncEnumerator(CancellationToken cancellationToken)
    {
        return new CustomAsyncDataEnumerator(cancellationToken);
    }
 
    private class CustomAsyncDataEnumerator : IAsyncEnumerator<int>
    {
        private readonly CancellationToken _ct;
        private readonly Random _rnd;
        private int _index;
 
        public int Current { get; private set; }
 
        public CustomAsyncDataEnumerator(CancellationToken ct)
        {
            this._rnd = new Random();
            this.Current = this._rnd.Next(100);
            this._ct = ct;
            this._index = 0;
        }
 
        public async ValueTask<bool> MoveNextAsync()
        {
            if (10 < this._index)
                return false;
 
            this._ct.ThrowIfCancellationRequested();
 
            // emulator
            await Task.Delay(200, this._ct).ConfigureAwait(false);
            this.Current = this._rnd.Next(100);
 
            this._index++;
 
            return true;
        }
 
        public ValueTask DisposeAsync()
        {
            Console.WriteLine("CustomAsyncDataEnumerator > DisposeAsync");
            return new ValueTask(Task.CompletedTask);
        }
    }
}

public class CustomAsyncDataProvider : IAsyncEnumerable<int>
{
    public IAsyncEnumerator<int> GetAsyncEnumerator(CancellationToken cancellationToken)
    {
        return new CustomAsyncDataEnumerator(cancellationToken);
    }
 
    private class CustomAsyncDataEnumerator : IAsyncEnumerator<int>
    {
        private readonly CancellationToken _ct;
        private readonly Random _rnd;
        private int _index;
 
        public int Current { get; private set; }
 
        public CustomAsyncDataEnumerator(CancellationToken ct)
        {
            this._rnd = new Random();
            this.Current = this._rnd.Next(100);
            this._ct = ct;
            this._index = 0;
        }
 
        public async ValueTask<bool> MoveNextAsync()
        {
            if (10 < this._index)
                return false;
 
            this._ct.ThrowIfCancellationRequested();
 
            // emulator
            await Task.Delay(200, this._ct).ConfigureAwait(false);
            this.Current = this._rnd.Next(100);
 
            this._index++;
 
            return true;
        }
 
        public ValueTask DisposeAsync()
        {
            Console.WriteLine("CustomAsyncDataEnumerator > DisposeAsync");
            return new ValueTask(Task.CompletedTask);
        }
    }
}

JakenVeina•15mo ago

async disposable and non-async disposal shouldn't be treated as separate operations implement one or the other, not both

mtreit•15mo ago

Yeah blindly implementing GC.SuppressFinalize without a finalizer kind of drives me nuts

JakenVeina•15mo ago

I'm guessing maybe there's a finalizer on the Disposable base class? but GC.SuppressFinalize() is still incorrect here

SWEETPONY•15mo ago

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

JakenVeina•15mo ago

right except for the first part don't implement both if your class implements IAsyncDisposable and the consumder doesn't call DisposeAsync() yes, that's a resource leak

mtreit•15mo ago

What is this code even disposing?

JakenVeina•15mo ago

the same way it's a resource leak if your class implements IDisposable and the consumer doesn't call Dispose() it's a defect in the consumer, not a defect in design your class should state its requirements, and if you have async work to do during disposal, you have async work to do during disposal, and consumers need to account for that

mtreit•15mo ago

Which is extremely rare

JakenVeina•15mo ago

there's a reason ServiceProvider.Dispose() will throw an exception if any registered services implement IAsyncDisposable ServiceProvider is a rare exception to what I said about implementing both, because SerivceProvider legitimately might or might not have async disposal work to do depends on what services are wrapped up in it yes, agreed

mtreit•15mo ago

I have a bit of a pet peeve about people over-using async without understanding what it's good for Since by itself it just makes your code slower

JakenVeina•15mo ago

nod use async/await if you have async work to do

mtreit•15mo ago

If you are doing a lot of concurrency with I/O it's amazingly useful

SWEETPONY•15mo ago

so IAsyncDisposable is useful sometimes thanks for review

mtreit•15mo ago

I'm sure it has it's use cases. I've never needed it myself.

JakenVeina•15mo ago

per docs

When developing an asynchronous enumerator that owns unmanaged resources. Asynchronous enumerators are used with the C# 8.0 async streams feature. For more information about async streams, see Tutorial: Generate and consume async streams using C# 8.0 and .NET Core 3.0. When your class owns unmanaged resources and releasing them requires a resource-intensive I/O operation, such as flushing the contents of an intermediate buffer into a file or sending a packet over a network to close a connection.

mtreit•15mo ago

Even then you need to really care about concurrency and thread pool usage to even go there Which might certainly be a thing for high-scale web services or the like But I work on high scale web services and I haven't needed this yet My enumerators don't usually own unmanaged resources

SWEETPONY•15mo ago

thanks for advices @JakenVeina @mtreit I appreciate that, thanks will change logic a bit and maybe remove this class at all I will check this

Gaming

Programming

✅ is it a good disposable class?