CODEWITHSUNDEEP
c#.net.net-coredependency-injectionidisposable
Matheus Lemos
Matheus Lemos

Reputation: 628

.NET core - Dependency Injection, factories and IDisposable

I'm investigating a memory leak in my application. Here's the context:

Let's assume I have to process different types of XMLs files and I receive a large amount of XML files per day, so I have a IXmlProcessor interface .

public interface IXmlProcessor
{
     void ProcessXml(string xml);
}

And some concrete XMLProcessors.

public class UserXmlProcessor : IXmlProcessor
{
     private readonly IUserRepository _userRepository;

     public UserXmlProcessor(IUserRepository userRepository)
     {
           _userRepository = userRepository;
     }

     public void ProcessXml(string xml)
     {
           // do something with the xml
           // call _userRepository 
     }
 }

All IXmlProcessor concrete types are registered to the DI Container, and to resolve them I have a Factory class, that's also registered to the DI container, something like this:

public class XmlProcessorFactory where TType : class
{
    private readonly IServiceProvider _serviceProvider;

    public XmlProcessorFactory(IServiceProvider serviceProvider)
    {
        _serviceProvider = serviceProvider;
    }

    public IXmlProcessor GetImplementation(string identifier)
    {
        var type = FindType(identifier);

        return _serviceProvider.GetService(type) as IXmlProcessor;
    }

    private Type FindType(string identifier)
    {
        // do some reflection to find the type based on the identifier (UserXmlProcessor, for example)
        // don't worry, there's caching to avoid unecessary reflection
    }
}

And at some point I call them all:

public class WorkItem
{
    public string Identifier { get; set; }
    public string Xml { get; set; }
}

public class WorkingClass
{

    private readonly XmlProcessorFactory _xmlProcessorFactory;

    public WorkingClass(XmlProcessorFactory xmlProcessorFactory)
    {
        _xmlProcessorFactory = xmlProcessorFactory;
    }

    public void DoWork(WorkItem item)
    {
        var processor = _xmlProcessorFactory.GetImplementation(item.Identifier);
        processor.ProcessXml(item.Xml);
    }
}

IUserRepository is a straightforward implementation, with an Entity Framwork context.

So, here's the problem: according to the Microsoft documentation:

Services resolved from the container should never be disposed by the developer.

Receiving an IDisposable dependency via DI doesn't require that the receiver implement IDisposable itself. The receiver of the IDisposable dependency shouldn't call Dispose on that dependency.

So if I inject IUserRepository to a controller, that's fine, the container will handle the object's disposal as well as the EF Context's disposal, none needs to be IDisposable.

But what about my Xml Processors? The documentation says:

Services not created by the service container

The developer is responsible for disposing the services.

Avoid using the service locator pattern. For example, don't invoke GetService to obtain a service instance when you can use DI instead. Another service locator variation to avoid is injecting a factory that resolves dependencies at runtime. Both of these practices mix Inversion of Control strategies.

And also _ = serviceProvider.GetRequiredService<ExampleDisposable>(); being an anti-pattern. But as you can see I do need to resolve the dependency at runtime, based on the XML Identifier and I dont' want to resort to switch cases.

So:

Upvotes: 4

Views: 2535

Answers (1)

Steven
Steven

Reputation: 172606

And also _ = serviceProvider.GetRequiredService<ExampleDisposable>(); being an anti-pattern.

This statement is too simplistic. Calling GetRequiredService is not an implementation of the Service Locator anti-pattern when called from within the Composition Root, and is, therefore, fine. When called outside the Composition Root, it is an implementation of the Service Locator anti-pattern. Most downsides that calling GetRequiredService has only exist when used outside the Composition Root.

Should the IXmlProcessors implement IDisposable and release IUserRepository manually?

No. The Microsoft Documentation is right. When your IUserRepository is resolved from the container, the Container will ensure it (or its dependencies) gets disposed of. Adding dispose logic in IUserRepository's consumers to dispose the repository only leads to unneeded complexity in the consumer. The dependency would only get disposed twice.

Should I also cascade and make IUserRepository implement IDisposable to release EntityContext?

No. When the EntityContext is managed by the DI Container, again, it will ensure it gets disposed of. So IUserRepository implementations should not implement disposal just to ensure that EntityContext gets disposed of. The Container will do this.

If so, wouldn't that affect the service lifetime if it's injected in a controller?

One of the problems with implementing IDisposable on the consumer is that this ripples through the system. Making a low-level dependency disposable, will force you to make all consumers in the dependency chain disposable as well. Not only does this lead to (unneeded) complexity in the consumers, it forces many classes in the system to be updated. This also means tests required to be added for all these classes. This would be a typical example of an Open/Closed Principle violation.

Do note that with the default .NET Core DI Container, it is very easy to accidentally cause memory leaks. This will happen when you resolve disposable Scoped or Transient components directly from the root container, instead of resolving them from a IServiceScope. Especially disposable Transient components are nasty, because at first it seems to work (as you always get a new instance), but those disposable Transients will be kept alive until the Container itself is disposed of, which will typically only happen on application shutdown.

So make sure you always resolve from a service scope, never from the root container (except when you run a short-lived (console) application).

Upvotes: 5

Related Questions