C#•3y 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; }           
}

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?

mtreit•5/4/23, 5:16 AM

Why would you ever need an async dispose?

Mmtreit Why would you ever need an async dispose?

SWEETPONYOP•5/4/23, 5:19 AM

because I want asynchronous disposal operation

mtreit•5/4/23, 5:19 AM

Why?

mtreit•5/4/23, 5:21 AM

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

JakenVeina•5/4/23, 5:22 AM

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

JakenVeina•5/4/23, 5:23 AM

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•5/4/23, 5:23 AM

logging strikes me as a good scenario

JakenVeina•5/4/23, 5:24 AM

in fact, I think that's the one time I literally implemented that interface

JakenVeina•5/4/23, 5:24 AM

for a Microsoft.Extensions.LoggingMicrosoft.Extensions.Logging implementation

SWEETPONYOP•5/4/23, 5:24 AM

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•5/4/23, 5:25 AM

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

JJakenVeina for a `Microsoft.Extensions.Logging` implementation

mtreit•5/4/23, 5:25 AM

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

JakenVeina•5/4/23, 5:25 AM

for sure, it's rare

JakenVeina•5/4/23, 5:26 AM

the way most people are going to encounter it is via ServiceProviderServiceProvider

JakenVeina•5/4/23, 5:26 AM

from Microsoft.Extensions.DependencyInjectionMicrosoft.Extensions.DependencyInjection

SSWEETPONY when working with a resource that may be shared across multiple threads (such as...

mtreit•5/4/23, 5:26 AM

I question whoever wrote that.

mtreit•5/4/23, 5:27 AM

async does not help prevent deadlocks

JakenVeina•5/4/23, 5:27 AM

it's a nicer way to handle thread signaling, arguably, than locking or semaphores or whatever, but yeah

JakenVeina•5/4/23, 5:28 AM

async code doesn't prevent deadlocks any more than locking or signaling does

mtreit•5/4/23, 5:31 AM

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

Mmtreit I question whoever wrote that.

SWEETPONYOP•5/4/23, 5:32 AM

I wrote it for asynchronous operations as I said

JakenVeina•5/4/23, 5:32 AM

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

SSWEETPONY I wrote it for asynchronous operations as I said

mtreit•5/4/23, 5:32 AM

But why?

JakenVeina•5/4/23, 5:33 AM

and I definitely don't buy the public OnDisposeAsyncpublic OnDisposeAsync property

mtreit•5/4/23, 5:33 AM

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

JakenVeina•5/4/23, 5:34 AM

and there's no parameter on DisposeAsyncCore()DisposeAsyncCore() indicating whether this is managed or unmanaged disposale

JakenVeina•5/4/23, 5:34 AM

and there's no finalizer defined, so GC.SuppressFinalize(this)GC.SuppressFinalize(this) is worthless

JakenVeina•5/4/23, 5:35 AM

and IsDisposedAsyncIsDisposedAsync is kinda nonsense

Mmtreit But why?

SWEETPONYOP•5/4/23, 5:35 AM

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•5/4/23, 5:35 AM

async disposable and non-async disposal shouldn't be treated as separate operations

JakenVeina•5/4/23, 5:35 AM

implement one or the other, not both

mtreit•5/4/23, 5:35 AM

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

JakenVeina•5/4/23, 5:36 AM

I'm guessing maybe there's a finalizer on the DisposableDisposable base class?

JakenVeina•5/4/23, 5:36 AM

but GC.SuppressFinalize() is still incorrect here

JJakenVeina async disposable and non-async disposal shouldn't be treated as separate operati...

SWEETPONYOP•5/4/23, 5:36 AM

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•5/4/23, 5:37 AM

right

JakenVeina•5/4/23, 5:37 AM

except for the first part

JakenVeina•5/4/23, 5:37 AM

don't implement both

JakenVeina•5/4/23, 5:37 AM

if your class implements IAsyncDisposableIAsyncDisposable and the consumder doesn't call DisposeAsync()DisposeAsync()

JakenVeina•5/4/23, 5:37 AM

yes, that's a resource leak

SSWEETPONY for smth like this: ```cpp public class CustomAsyncDataProvider : IAsyncEnumerab...

mtreit•5/4/23, 5:37 AM

What is this code even disposing?

JakenVeina•5/4/23, 5:38 AM

the same way it's a resource leak if your class implements IDisposableIDisposable and the consumer doesn't call Dispose()Dispose()

JakenVeina•5/4/23, 5:38 AM

it's a defect in the consumer, not a defect in design

JakenVeina•5/4/23, 5:38 AM

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

JJakenVeina your class should state its requirements, and if you have async work to do durin...

mtreit•5/4/23, 5:39 AM

Which is extremely rare

JakenVeina•5/4/23, 5:39 AM

there's a reason ServiceProvider.Dispose()ServiceProvider.Dispose() will throw an exception if any registered services implement IAsyncDisposableIAsyncDisposable

JakenVeina•5/4/23, 5:40 AM

ServiceProviderServiceProvider is a rare exception to what I said about implementing both, because SerivceProviderSerivceProvider legitimately might or might not have async disposal work to do

JakenVeina•5/4/23, 5:40 AM

depends on what services are wrapped up in it

Mmtreit Which is extremely rare

JakenVeina•5/4/23, 5:40 AM

yes, agreed

mtreit•5/4/23, 5:41 AM

I have a bit of a pet peeve about people over-using async without understanding what it's good for

✅ is it a good disposable class?

Similar Threads

Similar Threads

Similar Threads