Please properly dispose old knowledge

Greetings!  I have a confession to make.  When I learn something, I tend to expect that knowledge to remain inviolate and don't continually check to be sure that it hasn't changed.  This is a Very Bad Thing®©^TM in the programming world.  Coding conventions and guidance change all the time.  Still, as humans, we are loath to replace old, time worn techniques with new ones.

Disposing done wrong

For example, I learned from the .NET Framework 2.0's documentation that the IDisposable pattern consists of a "public void Dispose()" method, a "protected void Dispose(bool disposing)" method, and a finalizer.  The first method calls the second with true and then removes the object from the finalization queue.  The last method calls the second with false.  All actual disposing happens in the protected method.

using System;
public class ResourceUser: IDisposable
{
    bool m_disposed;
    ~ResourceUser()
    {
        Dispose(false);
    }
    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize();
    }
    protected void Dispose(bool disposing)
    {
        if (m_disposed) return;
        if (disposing)
        {
            //clean up managed resources
        }
        //clean up unmanaged resources
        m_disposed = true;
    }
}

Pretty simple, right?  Except that it's totally and utterly wrong and evil.

Will o'the Disp[ose]

Well, at least the finalizer part is.  If you read the latest Microsoft guidance you'll see that there is more complete information on the finalizer and its use, not to mention that it makes it a bit more clear than previously that the finalizer hook is necessary only to clean up unmanaged resources.  If your class includes only managed resources that must be disposed, you should not have a finalizer override.  Rather, just create the two Dispose method overloads to push through the dispose call and clean up on command.

Finally - let's get it right

The reason for this is rather simple - adding a finalizer increases the load on garbage collector significantly.  Creating an instance of an object with a finalizer adds it to the global finalization queue so that its memory will not be collected until the finalizer has executed.  Since the timing of this is non-deterministic, it means that your theoretically disposable objects will remain active in memory for longer than they have to when dispose is not properly called. Overall, this greatly increases management overhead.

So - the right way to go about it is not to create a finalizer, like so:

using System;
public class ResourceUser: IDisposable
{
    bool m_disposed;
    public void Dispose()
    {
        Dispose(true);
    }
    protected void Dispose(bool disposing)
    {
        if (m_disposed) return;
        if (disposing)
        {
            //clean up managed resources
        }
        m_disposed = true;
    }
}

Note that I also removed the comment "clean up unmanaged resources" after the "if (disposing)" code block.  This is because you should not take this shortcut if your class specifically includes unmanaged resources. Of course, most classes actually shouldn't include unmanaged resources - you should wrap them in a very thin, directed subclass of SafeHandle and then use an instance of that in your class.  The SafeHandle classes deal with the unmanaged cleanup and the majority of your classes never need to see the broad side of a finalizer again.

If you're working in a multithreaded environment, you may want to add some extra locking around the inner workings of your protected Dispose method, of course, but I wanted to leave something as an exercise for the reader.

Happy coding!