Skip to content

DependencyInjection does not allow lazy/factory injection of an IDisposable which should not be disposed #36049

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

Closed
binki opened this issue Feb 21, 2020 · 9 comments

Comments

@binki
Copy link

binki commented Feb 21, 2020

Describe the bug

I should be able to call IServiceCollection.AddSingleton<T>(Func<IServiceProvider, T> factory) such that T is IDisposable but prevent the service provider from calling Dispose() itself. Currently, the only way to do that is to use the instance overload which doesn’t support lazy/deferred calls: IServiceCollection.AddSingleton<T>(T instance).

To Reproduce

Steps to reproduce the behavior:

  1. Using version '3.1.2' of package 'Microsoft.Extensions.DependencyInjection'
  2. Run this code:
using Microsoft.Extensions.DependencyInjection;
using System;

class Program
{
    static void Main()
    {
        var factory = new DefaultServiceProviderFactory();
        var services = factory.CreateBuilder(new ServiceCollection());
        services.AddSingleton<ComplexDisposable>();
        // No way to opt out of automatic Dispose() call when providing a factory here.
        services.AddSingleton(sp => sp.GetRequiredService<ComplexDisposable>().DisposableMember);
        var serviceProvider = factory.CreateServiceProvider(services);
        using var disposeServiceProvider = serviceProvider as IDisposable;
        var disposableInstance = serviceProvider.GetRequiredService<Disposable>();
    }
}

class Disposable : IDisposable
{
    bool disposed;
    void IDisposable.Dispose()
    {
        if (disposed) throw new ObjectDisposedException(GetType().Name);
        disposed = true;
    }
}

class ComplexDisposable : IDisposable
{
    public Disposable DisposableMember { get; } = new Disposable();
    void IDisposable.Dispose()
    {
        using (DisposableMember)
        {
        }
    }
}
  1. See error:
System.ObjectDisposedException:
   at Disposable.System.IDisposable.Dispose() in C:\Users\binki\source\repos\DependencyInjectionDoubleDispose\DependencyInjectionDoubleDispose\Program.cs:line 25
   at ComplexDisposable.System.IDisposable.Dispose() in C:\Users\binki\source\repos\DependencyInjectionDoubleDispose\DependencyInjectionDoubleDispose\Program.cs:line 36
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.ServiceProviderEngineScope.Dispose() in /_/src/DependencyInjection/DI/src/ServiceLookup/ServiceProviderEngineScope.cs:line 71
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.ServiceProviderEngine.Dispose() in /_/src/DependencyInjection/DI/src/ServiceLookup/ServiceProviderEngine.cs:line 70
   at Microsoft.Extensions.DependencyInjection.ServiceProvider.Dispose() in /_/src/DependencyInjection/DI/src/ServiceProvider.cs:line 99
   at Program.Main() in C:\Users\binki\source\repos\DependencyInjectionDoubleDispose\DependencyInjectionDoubleDispose\Program.cs:line 16

Expected behavior

I expect there to be an API so that when I want to provide a type via a factory I can specify that the type should not be Disposed even though it implements IDisposable for the scenario where my factory gets the service from another process which is already managing the lifetime of the service.

Additional context

Include the output of dotnet --info:

C:\Users\binki>dotnet --info
.NET Core SDK (reflecting any global.json):
 Version:   3.1.101
 Commit:    b377529961

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.19551
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\3.1.101\

Host (useful for support):
  Version: 3.1.1
  Commit:  a1388f194c

.NET Core SDKs installed:
  2.1.200 [C:\Program Files\dotnet\sdk]
  2.1.201 [C:\Program Files\dotnet\sdk]
  2.1.202 [C:\Program Files\dotnet\sdk]
  2.1.400 [C:\Program Files\dotnet\sdk]
  2.1.401 [C:\Program Files\dotnet\sdk]
  2.1.402 [C:\Program Files\dotnet\sdk]
  2.1.403 [C:\Program Files\dotnet\sdk]
  2.1.500 [C:\Program Files\dotnet\sdk]
  2.1.502 [C:\Program Files\dotnet\sdk]
  2.1.503 [C:\Program Files\dotnet\sdk]
  2.1.504 [C:\Program Files\dotnet\sdk]
  2.1.505 [C:\Program Files\dotnet\sdk]
  2.1.507 [C:\Program Files\dotnet\sdk]
  2.1.509 [C:\Program Files\dotnet\sdk]
  2.1.600-preview-009426 [C:\Program Files\dotnet\sdk]
  2.1.600-preview-009472 [C:\Program Files\dotnet\sdk]
  2.1.600-preview-009497 [C:\Program Files\dotnet\sdk]
  2.1.600 [C:\Program Files\dotnet\sdk]
  2.1.601 [C:\Program Files\dotnet\sdk]
  2.1.602 [C:\Program Files\dotnet\sdk]
  2.1.604 [C:\Program Files\dotnet\sdk]
  2.1.700-preview-009618 [C:\Program Files\dotnet\sdk]
  2.1.700 [C:\Program Files\dotnet\sdk]
  2.1.801 [C:\Program Files\dotnet\sdk]
  2.1.802 [C:\Program Files\dotnet\sdk]
  2.2.202 [C:\Program Files\dotnet\sdk]
  2.2.204 [C:\Program Files\dotnet\sdk]
  2.2.300 [C:\Program Files\dotnet\sdk]
  2.2.401 [C:\Program Files\dotnet\sdk]
  2.2.402 [C:\Program Files\dotnet\sdk]
  3.1.101 [C:\Program Files\dotnet\sdk]

.NET Core runtimes installed:
  Microsoft.AspNetCore.All 2.1.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.4 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.5 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.11 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.13 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.15 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.2.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.2.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.2.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 2.1.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.3 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.9 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.11 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.13 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.15 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.2.3 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.2.5 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.2.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.2.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.2.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.1 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 2.0.7 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.0.9 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.2 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.3 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.4 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.5 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.6 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.7 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.8 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.9 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.11 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.13 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.15 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.2.3 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.2.5 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.2.6 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.2.7 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.2.8 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.1 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 3.1.1 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

To install additional .NET Core runtimes or SDKs:
  https://aka.ms/dotnet-download

C:\Users\binki>
@analogrelay analogrelay transferred this issue from dotnet/extensions May 7, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.Runtime untriaged New issue has not been triaged by the area owner labels May 7, 2020
@analogrelay analogrelay added this to the Future milestone May 7, 2020
@ViIvanov
Copy link
Contributor

Disposable from a sample breaks the IDisposable contract:

If an object's Dispose method is called more than once, the object must ignore all calls after the first one. The object must not throw an exception if its Dispose method is called multiple times.

@binki
Copy link
Author

binki commented May 19, 2020

@ViIvanov The exception is thrown to highlight that Dispose() is being called multiple times. The lifetime of ComplexDisposable.DisposableMember is meant to be managed by ComplexDisposable itself. The fact that DependencyInjection itself tries to dispose the injected object will render the object into an invalid state such that future uses of ComplexDisposable would encounter errors.

I would like to note that I personally disagree with the rule you point out. Correct code should only ever call IDisposable.Dispose() on an object once. If it is called multiple times, that means there are multiple objects trying to manage a shared object’s lifetime which may trigger race conditions or crashes due to the object being cleaned up prior to its consumers being done with it. Throwing on a second call to IDisposable.Dispose() is a way to help identify and fix this sort of bug.

However, I understand that I am going against the recommendation. I can make a more full-fledged adhering to the rule you point out if you desire. I will put that on my list and update this issue with that example if I can get around to it.

@ViIvanov
Copy link
Contributor

@binki The key point is it OK when Dispose() called multiple time for an instance (with a current guidelines for a .NET). I think the purpose for it is simplicity. If we will require to call Dispose() strictly once, we need also add property like bool IsDisposed to IDisposable interface to be able manage if instance already disposed or not. This just one thing of many.

But OK, now I get your point to be able manage a lifetime of IDisposable service even it created by a factory method.

@binki
Copy link
Author

binki commented May 21, 2020

Hello,

Here is the promised example of where Dispose() is called when it shouldn’t be which adheres to the guidance “The object must not throw an exception if its Dispose method is called multiple times.”:

using Microsoft.Extensions.DependencyInjection;
using System;

class Program
{
    static void Main()
    {
        var factory = new DefaultServiceProviderFactory();
        var services = factory.CreateBuilder(new ServiceCollection());
        using var complexDisposable = new ComplexDisposable();
        // No way to opt out of automatic Dispose() call when providing a factory here.
        services.AddSingleton(sp => complexDisposable.DisposableMember);
        var serviceProvider = factory.CreateServiceProvider(services);
        using (var disposeServiceProvider = serviceProvider as IDisposable)
        {
            var disposableInstance = serviceProvider.GetRequiredService<Disposable>();
            disposableInstance.DoSomething();
        }
        // Because we disposed the service provider, the instance of Disposable stored
        // at complexDisposable.DisposableMember will now be disposed. This is an invalid
        // state because we are managing the lifetime of complexDisposable.
        complexDisposable.DoSomethingComplicated();
    }
}

class Disposable : IDisposable
{
    bool disposed;
    void IDisposable.Dispose()
    {
        disposed = true;
    }
    public void DoSomething()
    {
        if (disposed) throw new ObjectDisposedException(GetType().Name);
    }
}

class ComplexDisposable : IDisposable
{
    bool disposed;
    public Disposable DisposableMember { get; } = new Disposable();
    void IDisposable.Dispose()
    {
        using (DisposableMember)
        {
            disposed = true;
        }
    }
    public void DoSomethingComplicated()
    {
        if (disposed) throw new ObjectDisposedException(GetType().Name);
        DisposableMember.DoSomething();
    }
}

Output:

Unhandled exception. System.ObjectDisposedException: Cannot access a disposed object.
Object name: 'Disposable'.
   at Disposable.DoSomething() in C:\Users\binki\source\repos\DependencyInjectionDoubleDispose\DependencyInjectionDoubleDispose\Program.cs:line 36
   at ComplexDisposable.DoSomethingComplicated() in C:\Users\binki\source\repos\DependencyInjectionDoubleDispose\DependencyInjectionDoubleDispose\Program.cs:line 53
   at Program.Main() in C:\Users\binki\source\repos\DependencyInjectionDoubleDispose\DependencyInjectionDoubleDispose\Program.cs:line 22

@ViIvanov
Copy link
Contributor

It is not a problem with calling Dispose() multiply times. It is more like accessing a member after it was disposed and managing services lifetime.

You are explicitly breaks the contract of the lifetime when your adds an "unmanaged" ComplexDisposable instance and "managed" depending Disposable.

@binki
Copy link
Author

binki commented May 21, 2020

My code structure ensures that ComplexDisposable lives longer than the composition container. The way my factory gets the instance it provides is irrelevant—the whole point of factories is to allow anything to be done to get the instance. In some cases, that might mean returning a singleton instance managed outside of the composition container which has a lifetime that lasts longer than that of the composition container. That is exactly what I provided an example of.

There are some workarounds, such as building an adapter/wrapper for the exported thing which shields its Dispose() method from the service provider/composition container. But that isn’t always an option and introduces complexity in providing the instance and/or consuming the instance.

@ericstj ericstj removed the untriaged New issue has not been triaged by the area owner label Jun 18, 2020
@davidfowl
Copy link
Member

Yes there's no way to avoid this today. We'd need a way to do this so that all other containers could participate.

Copy link
Contributor

Due to lack of recent activity, this issue has been marked as a candidate for backlog cleanup. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will undo this process.

This process is part of our issue cleanup automation.

@dotnet-policy-service dotnet-policy-service bot added backlog-cleanup-candidate An inactive issue that has been marked for automated closure. no-recent-activity labels Apr 14, 2025
Copy link
Contributor

This issue will now be closed since it had been marked no-recent-activity but received no further activity in the past 14 days. It is still possible to reopen or comment on the issue, but please note that the issue will be locked if it remains inactive for another 30 days.

@dotnet-policy-service dotnet-policy-service bot removed this from the Future milestone Apr 28, 2025
@github-actions github-actions bot locked and limited conversation to collaborators May 29, 2025
@dotnet-policy-service dotnet-policy-service bot removed no-recent-activity backlog-cleanup-candidate An inactive issue that has been marked for automated closure. labels May 29, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants