What is the right way to make sure Dispose is called on class member/field?

Question

I have a class (myClass), which has a class member (myDisposableMem) derived from IDisposable, thus having a Dispose() method. If it was a local variable, I can use using(...) {...} to make sure Dispose() will be called on this object. But it is a class member. What is the right way to make sure the Disposed is called on the member? I can think of 2 ways:

1) add a finallize() in the class, then call myDisposableMem.Dispose() inside

or

2) make my class inherit from IDisposible:

public class myClass : IDisposable
{
 ...
  public void Dispose()
  {
    myDisposableMem.Dispose();
  }
}
void main ()
{
  using (myClass myObj = new MyClass())
  {
   .... 
  }
}

Or maybe there is a better way?

possible duplicate of [How to implement IDisposable properly](http://stackoverflow.com/questions/2736414/how-to-implement-idisposable-properly) — Ed S., May 23 '12 at 00:12
Your object's `Dispose()` will be called by the garbage collector when it goes out of scope; so when the parent class goes out of scope, so does its members. — BeemerGuy, May 23 '12 at 00:13
@BeemerGuy.net: Only true if the class has a properly implemented finalizer. — Ilian, May 23 '12 at 00:44
@IlianPinzon -- you are correct; I mixed `Dispose()` with finalizers; my bad! Further reading: read Remarks section at http://msdn.microsoft.com/en-us/library/system.idisposable.dispose(v=vs.100).aspx — BeemerGuy, May 23 '12 at 00:50
Thanks everyone for your tips! I followed links and found this: [http://stackoverflow.com/questions/574019/calling-null-on-a-class-vs-dispose/574659#574659] answered by Jon Skeet. With that and your answers, I thinks the proper way is to declare the parent class as IDisposable and then also add the finalize to complement. — M W, May 24 '12 at 17:19

score 2 · Answer 1 · answered May 23 '12 at 14:52

An object or piece of code is said to "own" an IDisposable if it holds a reference, and there is no reason to believe that any other object or piece of code will call Dispose on it. Code which owns an an IDisposable held in a local variable must generally, before abandoning that variable, either call Dispose on it or else hand it off to some other code or object that expects to receive ownership. An object which owns IDisposable stored in a field must generally implement IDisposable itself; its Dispose method should check if the field is null and, if not, call Dispose on it.

Note that merely holding a reference to an IDisposable does not imply that one should call Dispose on it. One should only call Dispose on an IDisposable one holds if one has a reasonable expectation that no other code is going to do so.

Objects should override Finalize only if they would could perform some cleanup in a fashion that would be both useful and safe in an unknown threading context. In general, an object should not call Dispose on other IDisposable objects within a Finalize method, because although (contrary to what some sources claim) such objects are guaranteed to exist, one of the following conditions will most likely apply:

The other object may still be in use somewhere, in which case calling `Dispose` on it would be unsafe.
The other object may have already had its `Finalize` method called, in which case `Dispose` would be at best redundant, and may be unsafe.
The other object may already be scheduled to have its `Finalize` method run, in which case `Dispose` might be safe, but would likely be redundant.
The other object might not support thread-safe cleanup, in which case calling `Dispose` might not be redundant, but would be unsafe.
By the time the `Finalizer` could actually become eligible to run, anything that it might want to do might be moot (this scenario can come up with event subscriptions).

In short, if one owns an IDisposable, one should generally figure that if it can safely be cleaned up within a finalizer thread context, it will take care of itself, and if it can't, one shouldn't try to force it.

Objects should only override `Finalize` when they own _unmanaged_ resources that need to be cleaned up. Since the introduction of `SafeHandle` in .NET 2, this should never be necessary, since unmanaged resources should always be accessed through a `SafeHandle`-derived class. See Joe Duffy's blog "Never write a finalizer again" (http://www.bluebytesoftware.com/blog/PermaLink.aspx?guid=86c71425-57bc-4fcb-b34b-3262812f12cf). (That post does note that there are some advanced situations unrelated to resource management where a finalizer may still be useful.) — Bradley Grainger, May 24 '12 at 14:57
@BradleyGrainger: The terms "managed resource" and "unmanaged resource" are used by different people (including those at Microsoft) to mean slightly (but significantly) different things. By my definitions, an object holds an unmanaged resource if abandoning it without performing some cleanup action first would leave something else in a bad state. An object *is* a managed resource if it either holds managed resources, or it holds unmanaged resources and cleans them up via `Finalize`. Not all unmanaged resources can or should be wrapped in a `SafeHandle`. — supercat, May 24 '12 at 15:50
@BradleyGrainger: When not using `SafeHandle`, however, one should encapsulate the unmanaged resources into classes which are as small as practical, whose purpose is mainly to hold the resources and provide for their cleanup. In general, the total size of all objects to which a cleanup-finalizable object instance holds direct or indirect references should be bounded and predictable. Because this will often not be true of objects that hold managed resources, objects that hold managed resources should not have cleanup finalizers ("alarm" finalizers may be another matter). — supercat, May 24 '12 at 15:54

score 0 · Answer 2 · edited May 23 '17 at 12:33

The codes you post are for different purposes

public class myClass : IDisposable
{
  ...
  public void Dispose()
  {
    myDisposableMem.Dispose();
  }
}

implies that when someone (yourself or anybody else) calls the Dispose method of your class it will also call the dispose method on myDisposableMem which is the right thing to do (I highly encourage you that if there are other Disposable members on myClass you include them here as well...)

On the other hand

void main ()
{
  using (myClass myObj = new MyClass())
  {
    .... 
  }
}

implies that myObj ISN'T already a member of the class and that it will only be available inside the using statement and be later disposed.

These are two different scenarios and you have to identify which one applies to you. If you already have the member on the class I highly recommend you DON'T dispose it until myClass itself is Disposed, unless you are absolutely sure no one else will use it AND it takes a considerable amount of resources to just leave it there.

Some more insight on your actual code/scenario might help us tell you which approach you should use.

Also take into account how Finalize actually works, this other SO question might help you a bit there.

In my opinion, and I cannot guarantee I'm right here, the Dispose() method makes sure the implementing object frees all resources its using before becoming inaccessible to the application, but that does not remove it from the stack, whilst the finalize method is called, when implemented, during Garbage Collection.

score 0 · Accepted Answer · edited May 23 '17 at 12:06

Finalization and implementing IDisposable are not two separate options; they should always be done together. You should always implement IDisposable and clean up your resources in Dispose(), so that users can ensure that unmanaged resources are cleaned up in a timely fashion. And you Dispose() method should always finish off by calling GC.SuppressFinalize(this) to make sure its finalizer doesn't get called, for reasons that will be explained in a moment.

You should always implement ~MyObject() when you implement IDisposable. Its purpose is to give the garbage collector a way to ensure your object is always disposed of in case the people who are using it fail to take care of that themselves. If your finalizer is doing much more than calling this.Dispose() then you're probably using it incorrectly. Nope. See Ilian Pinzon's comments below, and I've apparently got some documentation to try reading again.

It's really just a safety net. Ideally, the finalizer should actually never be called, because objects are always explicitly disposed instead. When the garbage collector invokes it, it's forced to skip collecting that object. That in turn means that it gets passed along to the second garbage collector generation. Which in turn means that the object will stay in memory much longer, and ultimately be more expensive to clean up.

See here and here for more information on how it should be done.

That said, the best option is to get those disposable objects out of the class context and make them method variables instead whenever possible.

For example, if you've got a class that connects to a database, don't store a master instance of SqlConnection as a field. Instead, have your public methods create their own connections, and pass them to private methods as needed. This has two advantages: First, it means that you can declare your SqlConnections in using blocks and be done with it. Second, getting rid of that piece of 'global' state will help with thread safety, if you ever need it. (There are other advantages specific to `SqlConnection, but those are the general ones.)

"You should always implement ~MyObject() when you implement IDisposable." ... Not always. If your class does not have unmanaged members. You don't have to. — Ilian, May 23 '12 at 00:48
I think you misunderstand what finalizers are for. They are essentially a fallback mechanism for releasing unmanaged resources. If your class has a member that is disposable, then it's best to implement IDisposable in your class but not necessarily a finalizer. A finalizer is only warranted if your class directly owns an unmanaged resource (and even then, there are ways to solve this other than a finalizer). — Ilian, May 23 '12 at 03:27
See Jon Skeet's comments to the accepted answer in this question: http://stackoverflow.com/questions/3038571/whats-the-purpose-of-gc-suppressfinalizethis-in-dispose-method for example. — Ilian, May 23 '12 at 03:28
See this question as well: http://stackoverflow.com/questions/3882804/finalizer-and-idisposable — Ilian, May 23 '12 at 03:29
You should (almost) never need to write a finalizer in a C# class: http://www.bluebytesoftware.com/blog/PermaLink.aspx?guid=86c71425-57bc-4fcb-b34b-3262812f12cf — Bradley Grainger, May 23 '12 at 04:15

What is the right way to make sure Dispose is called on class member/field?

3 Answers3