Correct way of implementing Finalize and Dispose(When parent class implements IDisposable)

Question

I was implementing Finalize and Dispose in my classes, I implemented IDisposable on my parent class and override the Dispose(bool) overload in my child classes. I was not sure

1. whether to use a duplicate isDisposed variable(as its already there in base class) or not?
2. Whether to implement a finalizer in child class too or not?

Both these things are done in example given here -

http://guides.brucejmack.biz/CodeRules/FxCop/Docs/Rules/Usage/DisposeMethodsShouldCallBaseClassDispose.html

Whereas example in this MSDN article doesn't have any of these two - http://msdn.microsoft.com/en-us/library/b1yfkh5e.aspx

whereas this example in MSDN is not complete - http://msdn.microsoft.com/en-us/library/ms182330.aspx

Answer 1

It's very rare for a finalizer to be useful. The documentation you link to isn't totally helpful - it offers the following rather circular advice:

Implement Finalize only on objects that require finalization

That's an excellent example of begging the question, but it's not very helpful.

In practice, the vast majority of the time you don't want a finalizer. (One of the learning curves .NET developers have to go through is discovering that in most of the places they think they need a finalizer, they don't.) You've tagged this as (amongst other things) a WPF question, and I'd say it'd almost always be a mistake to put a finalizer on a UI object. (So even if you are in one of the unusual situations that turns out to require a finalizer, that work doesn't belong anywhere near code that concerns itself with WPF.)

For most of the scenarios in which finalizers seem like they might be useful, they turn out not to be, because by the time your finalizer runs, it's already too late for it to do anything useful.

For example it's usually a bad idea to try to do anything with any of the objects your object has a reference to, because by the time your finalizer runs, those objects may already have been finalized. (.NET makes no guarantees about the order in which finalizers run, so you simply have no way of knowing whether the objects you've got references to have been finalized.) It's bad idea to invoke a method on an object whose finalizer has already been run.

If you have some way of knowing that some object definitely hasn't been finalized, then it is safe to use it, but that's a pretty unusual situation to be in. (...unless the object in question has no finalizer, and makes use of no finalizable resources itself. But in that case, it's probably not an object you'd actually need to do anything to when your own object is going away.)

The main situation in which finalizers seem useful is interop: e.g., suppose you're using P/Invoke to call some unmanaged API, and that API returns you a handle. Perhaps there's some other API you need to call to close that handle. Since that's all unmanaged stuff, the .NET GC doesn't know what those handles are, and it's your job to make sure that they get cleaned up, at which point a finalizer is reasonable...except in practice, it's almost always best to use a SafeHandle for that scenario.

In practice, the only places I've found myself using finalizers have been a) experiments designed to investigate what the GC does, and b) diagnostic code designed to discover something about how particular objects are being used in a system. Neither kind of code should end up going into production.

So the answer to whether you need "to implement a finalizer in child class too or not" is: if you need to ask, then the answer is no.

As for whether to duplicate the flag...other answers are providing contradictory advice here. The main points are 1) you do need to call the base Dispose and 2) your Dispose needs to be idempotent. (I.e., it doesn't matter if it's called once, twice, 5 times, 100 times - it shouldn't complain if it's called more than once.) You're at liberty to implement that however you like - a boolean flag is one way, but I've often found that it's enough to set certain fields to null in my Dispose method, at which point that removes any need for a separate boolean flag - you can tell that Dispose was already called because you already set those fields to null.

A lot of the guidance out there on IDisposable is extremely unhelpful, because it addresses the situation where you need a finalizer, but that's actually a very unusual case. It means that lots of people write a IDisposable implementations that are far more complex than necessary. In practice, most classes call into the category Stephen Cleary calls "level 1" in the article that jpierson linked to. And for these, you don't need all the GC.KeepAlive, GC.SuppressFinalize, and Dispose(bool) stuff that clutters up most of the examples. Life's actually much simpler most of the time, as Cleary's advice for these "level 1" types shows.

Answer 2

Duplicate is needed

If you don't have any clean-up in child class simply call base.Dispose() and if there are some class level clean-up, do it after a call to base.Dispose(). You need to separate state of these two classes so there should be a IsDisposed boolean for each class. This way you can add clean-up code whenever you need.

When you determine a class as IDisposable, you simply tell GC I'm taking care of it's clean-up procedure and you should SuppressFinilize on this class so GC would remove it from it's queue. Unless you call GC.SupressFinalize(this) nothing happens special to an IDisposable class. So if you implement it as I mentioned there's no need for a Finilizer since you just told GC not to finalize it.

Answer 3

The correct way to implement IDisposable depends on whether you have any unmanaged resources owned by your class. The exact way to implement IDisposable is still something not all developers agree on and some like Stephen Cleary have strong opinions on the disposable paradigm in general.

see: Implementing Finalize and Dispose to Clean Up Unmanaged Resources

The documentation for IDisposable interface also explains this breifly and this article points out some of the same things but also on MSDN.

As far as whether a duplicate boolean field "isDisposed" is required in the base class. It appears that this is mainly just a useful convention that can be used when a subclass itself may add additional unmanaged resources that need to be disposed of. Since Dispose is declared virtual, calling Dispose on a subclass instance always causes that class's Dispose method to be called first which in turn calls base.Dispose as it's last step giving a chance to clear up each level in the inheritance hierarchy. So I would probably summarize this as, if your subclass has additional unmanaged resources above what is owned by the base then you will probably be best to have your own boolean isDisposed field to track it's disposal in a transactional nature inside it's Dispose method but as Ian mentions in his answer, there are other ways to represent an already-disposed state.

Answer 4

1) No need to duplicate

2) Implementing a finalizer will help to dispose items that are not explicitly disposed. But is not guaranteed. It is a good practice to do.

Answer 5

Only implement a finalizer if an object holds information about stuff needing cleanup, and this information is in some form other than Object references to other objects needing cleanup (e.g. a file handle stored as an Int32). If a class implements a finalizer, it should not hold strong Object references to any other objects which are not required for cleanup. If it would hold other references, the portion responsible for cleanup should be split off into its own object with a finalizer, and the main object should hold a reference to that. The main object should then not have a finalizer.

Derived classes should only have finalizers if the purpose of the base class was to support one. If the purpose of a class doesn't center around a finalizer, there's not much point allowing a derived class to add one, since derived classes almost certainly shouldn't (even if they need to add unmanaged resources, they should put the resources in their own class and just hold a reference to it).