The Dispose pattern, Finalizers, and Debug.Assert?

Peter Provost, Scott Densmore, and I were talking about some additions to our project coding guidelines, and I proposed a new one that we wanted to get some feedback on from the community.

According to the Dispose pattern, you’re supposed to create classes that look like this:

 public class BuildProcess : IDisposable
    {
        ~BuildProcess()
        {
            Dispose(false);
        }

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

        protected virtual void Dispose(bool disposing)
        {
            if (disposing)
            {
            }
        }
    }

Now the issue here is what should happen if your class is disposable and the finalizer gets called? Is this an error? I believe that it is. The only way for the finalizer to get called for this class is for me to have forgotten to dispose of the instance when I was finished with it. So my proposal for a change to the coding guidelines was to change the finalizer to have Debug.Assert() in it, so that I’ll know when I forget to call it.

What does the community think about this? Is this reasonable? Is there some deep, dark .Net secret that would prevent me from doing this? Any opinions greatly appreciated.

— bab

6 thoughts to “The Dispose pattern, Finalizers, and Debug.Assert?”

  1. I like introducing a "disposed" boolean that allows me to check if the object has already been disposed. You could use this with your Debug.Assert() idea as well. For example:

    void IDisposable.Dispose(bool disposing) {

    if (!disposed) {

    // clean up unmanaged resources

    if (disposing) {

    // clean up managed resources

    }

    disposed = true;

    }

    }

    ~MyClass()

    {

    Debug.Assert(disposed, "Object was not disposed before finalizer call");

    Dispose(false);

    }

  2. Hi Brian,

    The way I see is that the IDisposable is there as a way of advertising to clients that the server class is holding onto unmanged and/or limited/expensive resource. And then for the server class to offer the client a clean/convenient way of ensuring the Dispose method gets called (via the using statement). Whether its an error to skip the call to Dispose – it sounds as though its a good rule of thumb.

    I suppose a refinment to the recommendation could be that if the server is holding onto an expensive/limited resource raise an Assert, if the resource is unmanged but inexpensive then *consider* skipping the Assert on the basis that IDiposing is being offered more as a convenience and/or as a marker interface rather than a mandatory behaviour.

    I also agree with one of the other posts that a boolean instance variable indicating whether or not the object has had its Dispose method called should probably show up in the guideline.

    Thanks

    Christian

  3. if Dispose is called twice will the

    "GC.SuppressFinalize(this)" throw?

    If so should Dispose be more like this.

    public void Dispose()

    {

    if( !_disposed)

    {

    Dispose(true);

    GC.SuppressFinalize(this);

    }

    }

Leave a Reply

This site uses Akismet to reduce spam. Learn how your comment data is processed.