Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add IAsyncDisposable support in netstandard2.1 #1467

Closed
pengweiqhca opened this issue Apr 19, 2019 · 7 comments
Closed

Add IAsyncDisposable support in netstandard2.1 #1467

pengweiqhca opened this issue Apr 19, 2019 · 7 comments
Milestone

Comments

@pengweiqhca
Copy link

pengweiqhca commented Apr 19, 2019

@davidfowl
Copy link
Member

Yep, I'll take this one. I started looking but we need to do some small infrastructure work (see #1431)

natemcmaster pushed a commit to dotnet-maestro-bot/Common that referenced this issue Apr 22, 2019
@kevinchalet
Copy link

@davidfowl as part of this task, is there a chance you could use Microsoft.Bcl.AsyncInterfaces and backport all this async disposal love to .NET Standard 2.0? Both DI and Hosting could benefit from this ❤️

@davidfowl
Copy link
Member

Yea that’s the plan

@analogrelay analogrelay added this to the Backlog milestone Jun 27, 2019
@kevinchalet
Copy link

I see this was punted from 3.0. Would an external contribution help make it happen in time for 3.0? 😄

@kevinchalet
Copy link

Just in case... here's my attempt: #2137

@analogrelay
Copy link

Would an external contribution help make it happen in time for 3.0?

Not really, it's not about the code to be written, it's about the risk level of the change and the impact it would have on customers. We now have a high bar for changes to 3.0, mostly just ship-blocking issues, while we stabilize to prepare for the release in September. In general, we are only taking high-impact bug fixes for 3.0.

Are there specific scenarios you have that this will unblock? We'll likely take this change in some branch because it seems like a good thing, but we need to make a decision between 3.0 and 5.0. Since there isn't much time to stabilize and get feedback now, taking a change like this is fairly high risk right now.

@kevinchalet
Copy link

Are there specific scenarios you have that this will unblock?

Oh yeah: as a library writer, being able to use IAsyncDisposable services and making sure they'll be correctly disposed of by .NET Framework applications.

Consider this demo:

public static class Program
{
    public static void Main(string[] args)
    {
        var services = new ServiceCollection();
        services.AddSingleton<MyService>();

        var provider = services.BuildServiceProvider();

        _ = provider.GetRequiredService<MyService>();
        provider.Dispose();
    }
}

public class MyService : IAsyncDisposable
{
    public ValueTask DisposeAsync() => default;
}

On .NET Core, it will throw an exception, as you have to use provider.DisposeAsync() as soon as you have IAsyncDisposable services:

System.InvalidOperationException : ''MyService' type only implements IAsyncDisposable. Use DisposeAsync to dispose the container.'

Once you switch to DisposeAsync(), everything will work flawlessly and MyService.DisposeAsync() will be correctly called.

On .NET Framework, you'll get no exception and MyService.DisposeAsync() will never be called (which is terrible).

By adopting this change in time for 3.0, you'd make async disposables behave consistently between .NET Framework and .NET Core.

Given that you've adopted exactly the same approach in the ASP.NET Core connections abstractions and in the SignalR client without any issue, I'm tempted to think it's a fairly safe change 😄

@ghost ghost locked as resolved and limited conversation to collaborators Dec 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants