-
Notifications
You must be signed in to change notification settings - Fork 5k
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
Comments
|
@ViIvanov The exception is thrown to highlight that I would like to note that I personally disagree with the rule you point out. Correct code should only ever call 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. |
@binki The key point is it OK when But OK, now I get your point to be able manage a lifetime of |
Hello, Here is the promised example of where 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:
|
It is not a problem with calling You are explicitly breaks the contract of the lifetime when your adds an "unmanaged" |
My code structure ensures that There are some workarounds, such as building an adapter/wrapper for the exported thing which shields its |
Yes there's no way to avoid this today. We'd need a way to do this so that all other containers could participate. |
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. |
This issue will now be closed since it had been marked |
Describe the bug
I should be able to call
IServiceCollection.AddSingleton<T>(Func<IServiceProvider, T> factory)
such thatT
isIDisposable
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:
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
Dispose
d even though it implementsIDisposable
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
:The text was updated successfully, but these errors were encountered: