Implementing IDisposable for Entity Framework in custom class

Question

I use Entity Framework like

public sealed class CacheManagementHelper 
{
   private readonly GJobEntities db = new GJobEntities();

   public List<User> GetUsers()
   { 
       return db.Users.ToList();
   }
}

And MS Visual Studio 2019 suggests following

Warning CA1001 Implement IDisposable on 'CacheManagementHelper' because it creates members of the following IDisposable types: 'GJobEntities'. If 'CacheManagementHelper' has previously shipped, adding new members that implement IDisposable to this type is considered a breaking change to existing consumers.

I found some clue here

Entity Framework and calling context.dispose()

But it is still unclear if I have implement IDisposable.

Thanks for help!

Answer 1

But it is still unclear if I have implement IDisposable.

It's a tricky question with a couple of answers:

• No, you should not.
• Yes, you should.
• No, you don't have to.

In general: IDisposable objects should be disposed at the end of their lifetime. There is some debate on whether the framework handles this for you, but that's a faulty approach. (at the bottom, under The fauly approach, I'll add why)

The responsibility of disposing lies, in general, at the creator. So; if you create it, you dispose it.

This is where @swdon's comment come's in;

The biggest issue I see in his code is db is public. I don't know who is responsible for getting rid of correctly. – swdon Aug 2 '19 at 7:07

This is basically why you should not have public non-readonly IDisposable member fields. Because: you'll have no simple way of tracking it when the caller overwrites it and who's responsibe then to call Dispose?

---

No, you should not

IoC

So, you might want to use an IoC framework like Unity. What this IoC framework does is; it take over the responsibility of creating objects, and therefore, it's also responsible for the disposing of the objects. Do note: the purpose of an IoC framework is not about being a factory, it's just one of the side effects.

So, with IoC, you won't create the member yourself, you request it. It might look something like this:

private GJobEntities _context;

//constructor: this object can also be created by the IoC framework.
public CacheManagementHelper (GJobEntities context)
{
    //set your field here
    _context = context;
}

If you use this, you don't have to implement IDispossable here because you are not creating the object yourself, you just request the context from the framework and let that call the Dispose method.

Why do I mention this? Because it's the default of the future of .net.

So, using this approach;

No, you should not implement IDisposable because you didn't create IDisposables, (but you should get rid of the DbContext creation).

---

Yes, you should

IDisposable

If you need to stick to your current pattern - which makes sense - you should implement IDisposable. By creating IDispossable members, you are responsible for disposing them. You can delegate this to the creator of the class by implementing IDisposable.

public sealed class CacheManagementHelper : IDisposable

The creator of this object now creates the IDisposable object, and hence is responsible for disposing it.

Your only job is to implement the interface correctly and dispose your resources.

It's pretty though to implement it correctly from reading the docs. I would advise you to use this implementation: Proper use of the IDisposable interface

So, using this approach;

Yes, you should implement IDisposable and clean up your resources.

---

No, you don't have to

Alternatively,

You could create the context while calling the function, keep it local and dispose it there. By this you'll overcome the need to implement IDisposable, because the creator of the context is also disposing it.

//keeping it local
public List<User> GetUsers()
{ 
    using (var db = new GJobEntities())
        return db.Users.ToList();
}

So, using this approach;

No, you don't have to implement IDisposable, because you are not creating IDisposable member fields.

---

The faulty approach

If you read "Proper use of the IDisposable interface" you know that, if it's correctly implemented, you're pretty much safe guarded against memory leaks.

So, one might argue that you don't need to call Dispose all together because the garbage collector will do it for you.

This is a misconception.

The trouble with it is the following:

• 1) you don't know if the component you're calling is correctly implemented. It might leak if not being disposed (if using unmannaged memory).
• 2) memory leaks are; time related. Surprising hé? Look at it like this: the memory is back when you reboot the system or restart the application. It's not permanent. From the old days this was bound to the application lifetime span. But developments regarding to multi threading, serverless coding and so own, do require us to look at it slightly different. The leak can be relvant within a specific time frame. A good example of this is high memory consuming image processing or exhausting connection pools.

So, there is no way of telling when the garbage collector comes by and clean thing up. It might be not fast enough to create problems with: open connection counts, graphic resources etc. You're leaking, and rely on the garbage collector to fix your mess, which is a faulty approach.

---

final remark: a EF DbContext gather a lot of resources during it's life time by tracking all you changes (It's basically a unit of work). Keeping a long lived DbContext might cause a performance hit over time. So, just be aware of that.