Is it ok to call a virtual method from Dispose or a destructor?

Question

I can't find a reference to it but I remember reading that it wasn't a good idea to call virtual (polymorphic) methods within a destructor or the Dispose() method of IDisposable.

Is this true and if so can someone explain why?

Answer 1

Virtual methods are discouraged in both constructors and destructors.

The reason is more practical than anything: virtual methods can be overridden in any manner chosen by the overrider, and things like object initialization during construction, for example, have to be ensured lest you end up with an object that has random nulls and an invalid state.

Answer 2

To expand on Jon's answer, instead of calling virtual methods you should be overriding the dispose or the destructor on sub classes if you need to handle resources at that level.

Although, I don't believe there is a "rule" in regards to behavior here. But the general thought is that you want to isolate resource cleanup to only that instance at that level of the implementation.

Answer 3

I do not believe there is any recommendation against calling virtual methods. The prohibition you are remembering might be the rule against referencing managed objects in the finalizer.

There is a standard pattern that is defined the .Net documentation for how Dispose() should be implemented. The pattern is very well designed, and it should be followed closely.

The gist is this: Dispose() is a non-virtual method that calls a virtual method Dispose(bool). The boolean parameter indicates whether the method is being called from Dispose() (true) or the object's destructor (false). At each level of inheritance, the Dispose(bool) method should be implemented to handle any cleanup.

When Dispose(bool) is passed the value false, this indicates that the finalizer has called the dispose method. In this circumstance, only cleanup of unmanaged objects should be attempted (except in certain rare circumstances). The reason for this is the fact that the garbage collector has just called the finalize method, therefore the current object must have been marked ready-for-finalization. Therefore, any object that it references may also have been marked read-for-finalization, and since the sequence in non-deterministic, the finalization may have already occurred.

I highly recommend looking up the Dispose() pattern in the .Net documentation and following it precisely, because it will likely protect you from bizarre and difficult bugs!

Answer 4

Calling virtual methods from a finalizer/Dispose is unsafe, for the same reasons it is unsafe to do in a constructor. It is impossible to be sure that the derived class has not already cleaned-up some state that the virtual method requires to execute properly.

Some people are confused by the standard Disposable pattern, and its use of a virtual method, virtual Dispose(bool disposing), and think this makes it Ok to use any virtual method durring a dispose. Consider the following code:

class C : IDisposable {
    private IDisposable.Dispose() {
        this.Dispose(true);
    }
    protected virtual Dispose(bool disposing) {
        this.DoSomething();
    }

    protected virtual void DoSomething() {  }
}
class D : C {
    IDisposable X;

    protected override Dispose(bool disposing) {
        X.Dispose();
        base.Dispose(disposing);
    }

    protected override void DoSomething() {
        X.Whatever();
    }
}

Here's what happens when you Dispose and object of type D, called d:

1. Some code calls ((IDisposable)d).Dispose()
2. C.IDisposable.Dispose() calls the virtual method D.Dispose(bool)
3. D.Dispose(bool) disposes of D.X
4. D.Dispose(bool) calls C.Dispose(bool) statically (the target of the call is known at compile-time)
5. C.Dispose(bool) calls the virtual methods D.DoSomething()
6. D.DoSomething calls the method D.X.Whatever() on the already disposed D.X
7. ?

Now, most people who run this code do one thing to fix it -- they move the base.Dispose(dispose) call to before they clean-up their own object. And, yes, that does work. But do you really trust Programmer X, the Ultra-Junior Developer from the company you developed C for, assigned to write D, to write it in a way that the error is either detected, or has the base.Dispose(disposing) call in the right spot?

I'm not saying you should never, ever write code that calls a virtual method from Dispose, just that you'll need to document that virtual method's requirement that it never use any state that's defined in any class derived below C.