Do we really need to implement IDisposable in Repository or UnitOfWork classes?

Question

First, let's see what Microsoft says about Asp.Net Core's default Dependency Injection services:

The framework takes on the responsibility of creating an instance of the dependency and disposing of it when it's no longer needed.

https://docs.microsoft.com/en-us/aspnet/core/fundamentals/dependency-injection?view=aspnetcore-2.1#disposal-of-services

I.e. the framework would be calling a classes Dispose method (assuming the class implements IDisposable)

Second, the DbContext class does indeed implement IDisposable out of the box.

Third, in our Startup.cs class, we add our DbContext via the AddDbContext method, which by default gets added as a Scoped instance (i.e. our DbContext is created and garbage collected on each single request).

Scoped lifetime services are created once per request.

https://docs.microsoft.com/en-us/aspnet/core/fundamentals/dependency-injection?view=aspnetcore-2.1#service-lifetimes

E.g.

public void ConfigureServices(IServiceCollection services)
{
    services
        .AddDbContext<TheStoreDbContext>(ConfigureDbContext)      
        .AddMvc().SetCompatibilityVersion(CompatibilityVersion.Version_2_2) 
}

Conclusion, we don't need to explicitly call context.Dispose() anywhere in our Asp.net Core application.

So why are there so many examples online and in tutorials which show you that you have to implement IDisposable in your Repository or UnitOfWork classes?

E.g.

public class UnitOfWork : IUnitOfWork
    {
        private readonly DbContext _context;

        public IProductRepository ProductRepository { get; }

        public UnitOfWork(DbContext context)
        {
            _context = context;
            ProductRepository = new ProductRepository(context);
        }

        public void Dispose()
        {
            _context.Dispose();
        }
    }

What do you think? Is this a valid question? Does it make sense not to explicitly call the Dispose() method anywhere?

Do you have to? No, I don't for the same reason - I leave the framework clean up, but then again the applications I use ASP.NET Core in are not performance or memory critical. If you have a lot more going on then it is good to implement IDisposable to deconstruct your resources. Is leaving cleanup for the framework good practice, would that fly by in C++? Not really, you'd run into problems over time. — ColinM
If you using DI, typically you'd let the DI framework dispose of your object for you. You need to set to the correct lifestyle to prevent memory leaks, so I always avoid this. Any non injected dependencies though should be cleaned up yourself. The Dispose call is still important, it just depends who calls it, you or the DI framework. — Liam
Possible duplicate of How to dispose resources with dependency injection — Liam
the Tl;Dr is if a dependency implements IDisposable you should always call Dispose. How you do that is up to you. — Liam
Although I am sure (as hinted by ColinM's coment) that there are indeed performance-critical cases where you'd want to manually Dispose a DbContext instance, the "reason" why people do it is often because it's just blindly copied from someone else Cargo-Cult Style :) Also, as regards the UoW/Repository example - DbContext is already providing that (the context is a Unit of Work and DBSet<T> is a Repository) so any examples you see like the one you have in the question may not necessarily represent best practice :) — Stephen Byrne

Steven Steven · Accepted Answer · 2019-01-29T16:35:56

Your example of the UnitOfWork class actually applies a bad practice. The rule is that a class should only dispose what it owns. In the case of the UnitOfWork, however, it does not own the DbContext as that is not created by UnitOfWork, but instead provided by someone else.

Letting UnitOfWork dispose of that DbContext is even problematic, because UnitOfWork has no way of knowing whether or not DbContext is still used by other classes in the system when UnitOfWork is disposed. In other words, the system might break when UnitOfWork starts disposing that dependency.

So if you follow the rule of only disposing what you own, it means that UnitOfWork should in fact not implement IDisposable at all. And this means that you let 'someone else' control the lifetime of the DbContext.

This idea of moving the control over the lifetime of dependencies (such as DbContext) to a third party is not something new, and certainly not something specific to the new Microsoft DI container. This is in fact an old idea, which is the idea that you centralize the control over the lifetime of application components in the start-up path of the application. In the context of DI, this start-up path is typically referred to as the Composition Root.

Inside the Composition Root, it will be the job of the Composer to create application components, manage their lifetime, and dispose of them when they are not needed any longer. A DI Container (like the .NET Core DI Container) acts as the Composer in your system, but you can do this just as well by hand—a practice commonly known as Pure DI.

Also see this answer

Do we really need to implement IDisposable in Repository or UnitOfWork classes?

1 Answers