Disposing Your Finalizers

  • Article

Yesterday I was bitten by a bug in our test infrastructure where the code was attempting to dispose of an object in the finalize method. It took me a bit longer then I had hoped to track down the issues, but in the end I did track it down and fixed it.

Here is some example code where the bug exists:

class Logger : IDisposable
{
    private StreamWriter Log { get; set; }

    public Logger(FileInfo file)
    {
        Log = File.CreateText(file.FullName);
    }

    public void Write(string msg)
    {
        Log.WriteLine(msg);
    }

    ~Logger()
    {
        Dispose();
    }

    public void Dispose()
    {
        if (Log != null)
        {
            Log.Dispose();
            Log = null;
        }
        GC.SuppressFinalize(this);
    }
}

You may be looking at this code and asking what is wrong even after you glance over the MSDN article, Overriding the Finalize Method. The first sentence of the article is, "A Finalize method acts as a safeguard to clean up resources in the event that your Dispose method is not called." At a quick glance one might think the following. A dispose method is designed to be called multiple times without adversely affecting the object and the framework has a notion of a Finalize method that will always be called, so why don't I just call the Dispose method from Finalize?

The reason that you never want to call a Dispose method from a Finalize method is that the Finalize method was implemented to ensure that all unmanaged resources are cleaned up since the GC cannot handle these. Since the GC is allowed to clean up/finalize managed resources in which ever order it wishes if you run the above code enough you will notice that the StreamWriter object is finalized before the Logger object. At this point when the Logger object is finalized it attempts to invoke it's Dispose method, which in turn attempts to invoke Dispose on StreamWriter which throws because the object has been cleaned up.

To fix this code I removed the Finalize method, since it should have never been implemented as we are never allocating any unmanaged resources.

Here are some other resources on the topic.