C#•2y 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•2y ago

Why would you ever need an async dispose?

SWEETPONYOP•2y ago

because I want asynchronous disposal operation

mtreit•2y ago

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

JakenVeina•2y ago

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

JakenVeina•2y 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•2y 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

SWEETPONYOP•2y 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•2y 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•2y ago

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

JakenVeina•2y ago

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

mtreit•2y ago

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

JakenVeina•2y 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•2y ago

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

SWEETPONYOP•2y ago

I wrote it for asynchronous operations as I said

JakenVeina•2y ago

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

mtreit•2y ago

But why?

JakenVeina•2y ago

and I definitely don't buy the public OnDisposeAsync property

mtreit•2y ago

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

JakenVeina•2y 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

SWEETPONYOP•2y 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•2y ago

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

mtreit•2y ago

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

JakenVeina•2y ago

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

SWEETPONYOP•2y 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•2y 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•2y ago

What is this code even disposing?

JakenVeina•2y 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•2y ago

Which is extremely rare

JakenVeina•2y 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•2y 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•2y ago

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

mtreit•2y ago

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

SWEETPONYOP•2y ago

so IAsyncDisposable is useful sometimes thanks for review

mtreit•2y ago

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

JakenVeina•2y 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•2y 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

SWEETPONYOP•2y 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?