dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.57k stars 4.55k forks source link

Dependency Injection: Conforming container tests missing a test for assumed provided instance disposal behaviour #102651

Open alistairjevans opened 1 month ago

alistairjevans commented 1 month ago

In the MS DI implementation, it is stated in docs that services not created by the service container are not disposed of when the application instance stops.

However, this is not the default Autofac behaviour, where services registered from instances are disposed of when the root container/service provider is disposed, unless explicitly switched off using .ExternallyOwned() on the registration.

Our Autofac MS DI integration currently follows Autofac defaults rather than the assumed behaviour of MS DI, leading to user confusion/frustration: https://github.com/autofac/Autofac.Extensions.DependencyInjection/issues/15.

The reason for the deviation from the assumed behaviour is simply that this default "provided instance" handling is not present in the conforming container tests.

This hypothetical test, specifically, passes on the MS DI container, but fails on our Autofac container:

[Fact]
public void ServiceInstancesRegisteredAreNotDisposedWhenTheProviderIsDisposed()
{
    // Issue 15: M.E.DI documents that provided instances are not disposed when the service provider is disposed,
    //            but there is no conformance test.
    var externalService = new DisposeTracker();
    var services = new ServiceCollection().AddSingleton(externalService);
    var rootProvider = CreateServiceProvider(services);
    ((IDisposable)rootProvider).Dispose();

    Assert.False(externalService.Disposed);
}

We've come to the decision to amend the Autofac behaviour to match the assumed behaviour of MS DI, but it would be great if this could be added to the formal conforming container specification via the additional test, to reduce user confusion when switching to an alternative container.

dotnet-policy-service[bot] commented 1 month ago

Tagging subscribers to this area: @dotnet/area-extensions-dependencyinjection See info in area-owners.md if you want to be subscribed.

ericstj commented 3 weeks ago

@alistairjevans would you be willing to create a PR to add this?

alistairjevans commented 3 weeks ago

Yep, happy to; I'm on leave right now, but I can raise it in a couple of weeks.