Throwing exception in finalizer to enforce Dispose calls:

Question

Here is the typical IDisposable implementation that I believe is recommended:

~SomeClass() {
    Dispose(false);
}

public void Dispose() {
    GC.SuppressFinalize(this);
    Dispose(true);
}

protected virtual void Dispose(bool isDisposing) {
    if(isDisposing) {
        // Dispose managed resources that implement IDisposable.
    }
    // Release unmanaged resources.
}

Now, to my understanding, the idea behind the finalizer there is that if I don't call Dispose, my unmanaged resources will be released properly still. However, to my knowledge, it is generally agreed upon that not calling Dispose on an object that implements IDisposable is probably a bug.

Is there a particular reason not to fully embrace this and do this instead?

~SomeClass() {
    throw new NotImplementedException("Dispose should always be called on this object.");
}

public virtual void Dispose() {
    GC.SuppressFinalize(this);

    // Dispose managed resources that implement IDisposable.

    // Release unmanaged resources.
}

Answer 1

From .NET 2.0 and on, An unhandled Exception thrown in a Finalizer causes the process to terminate if the default policy is not overridden.

To my understanding, a Finalizer is not an expected location where an Exception should be thrown. I think it is possible for a Dispose() method not to have been called for an unexpected reason (Thread abort, ...) from which a clean recovery is still possible, provided that the Finalizer executes properly.

Answer 2

Throwing an exception in a finalizer is almost always a bad idea.

Even though there may be situations where it would in fact achieve the desired result (causing the programmer to be notified of a problem without messing up anything else), a finalizer would have no way of knowing when that would be the case.

A better approach would be to have a method which will indicate (perhaps by throwing any exception) whether any objects have been wrongfully abandoned; one may call that method at the end of the program, or perhaps at times when new objects are created. This might cause an exception to get thrown in a part of the code which had nothing to do with the particular instance that was abandoned, but that would still likely be better than throwing the exception in a finalizer context.

Answer 3

Throwing an exception from a finalizer is one of the worst things you could ever do. It can cause several different kinds of behavior. The exception could cause the ExecutionEngine to crash thereby bringing down your entire process, you could block the finalizer causing an OutOfMemory (OOM) crash, etc. Don't forget that the finalizer runs on a single thread and is one of the most important threads - you don't want it to get screwed up.

Answer 4

Since Developer tend to become lazy and ignore whatever warning can be ignored and furthermore IIS7.5 applications completely ignore Debug.Fail by default I use the same pattern too. With one exception:

I do this only in debug build since it will not have any benefit to a release build. Furthermore it removes the slow finalizer from classes that do not hold unmanaged resources directly.

public void Dispose()
{
    // Do your dispose stuff here
#if DEBUG
    GC.SuppressFinalize(this);
}
~MyClass()
{   throw new ObjectNotDisposedException(); // This class is not intended to be used without Dispose.
#endif
}

There are cases where a missing dispose could really hog the application, e.g. a remote transaction context. Termination is nothing worse in this case and will get on lazy developers nerves. I always prefer to bring problems back to their source.