Best practice Dispose Pattern C#

Here is my take on the Best practice Dispose pattern for most situations. It is heavily influenced by Juval Löwy from IDesigns book “Programming .NET Components”, and some other sources which I no longer remember, since I made it a few years ago and have stuck to it ever since. Here it is:

public class Class1 : IDisposable
{
    private bool m_IsDisposed;

    ~Class1()
    {
        Dispose(false);
    }


    [MethodImpl(MethodImplOptions.Synchronized)]
    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    private void Dispose(bool isDisposing)
    {
        if (m_IsDisposed)
            return;

        if (isDisposing)
        {
            FreeManagedRessources();
        }
        FreeUnmanagedRessources();
        m_IsDisposed = true;
    }

    protected virtual void FreeManagedRessources()
    {
        //Free managed ressources here. Typically by calling Dispose on them
    }

    protected virtual void FreeUnmanagedRessources()
    {
        //Free unmanaged ressources here.
    }
} 

Another thing is that you can inherit the class and override the FreeManagedRessources and FreeUnmanagedRessources as needed.


Comments

Hi Jesper,

I think this code sample needs some changes. Microsoft has a template, but you have to consider some topics:

Patrick Peters
Mon, Jun 22, 2009

Hi Patrick,

Thank you for comment. Nice to get feedback :O)

First look at page 98 of Juval Lowys book “Programming .NET components.” I guess I am a fan of his. :O) The Dispose pattern presented by me is just an improved version of his Dispose Pattern (and nicer looking :O) ). He makes what he almost calls a “bulletproof” Dipose pattern. This of course also means that not all parts of it is necessary all the time. But it certainly helps.

I will answer each of your comments here:

“virtual (polymorphistic) mehods for releasing managed and unmanaged code should be called Dispose(bool disposed)”

“keep releasing methods for unmanaged and managed code(like “FreeManagedResources” and “FreeUnmanagedResources”) always private for the class itself, it is the responsibility of the class and not for derived classes”

“a finalizer is expensive to introduce. You only need them when the class holds unmanaged resources to be sure that unmanaged resources are always cleaned up by the GC.”

“[MethodImpl(MethodImplOptions.Synchronized)] is not needed by default.”

“use camelCasing instead of m_ for class fields (this is Microsoft’s default), but that’s just code style.”

Again thank you for the feedback. I didn’t understand your first point, but the rest made me think, even though I disagree. I haven’t been changing this pattern for a few years now and I think I will keep on using it. At least it has not given me any trouble. :O)

Jesper
Tue, Jun 23, 2009

Hi Jesper,

The template I am using is coming from Microsoft’s MSDN site and is also used by Bill Wagners “Effective C#: 50 Specific Ways to Improve Your C#“. IDesign’s coding standards are very useful and helpful but are not always in line with Microsoft’s guidelines. This also the fault of Microsof itself where there some gaps in the guidelines. The one and only coding guidelines reference that is used by all MS teams is the book “Framework Design Guidelines” and the Dispose template I am referring to is coming from that book also. MSDN is also referering to that book but can only state slices of the text from it due to legal/copyright stuff.

Here are my replies again, read the text after ->>>>>

This is the Dispose template I am using (and is coming from MS/Framework Design Guidelines)

public class MyHelper : IDisposable
{
    private bool disposed;

    public MyHelper()
    {
    }

    ~MyHelper()
    {
        Dispose(false);
    }

    #region IDisposable Members

    public void Dispose()
    {
        Dispose(true);

        GC.SuppressFinalize(this);
    }

    #endregion

    protected virtual void Dispose(bool isDisposing)
    {
        if (!this.disposed)
        {
            if (isDisposing)
            {
                //cleanup managed resources here, you may also chain it to a private method in this class
            }

            //cleanup unmanaged resources, you may chain it to a private method in this class

            this.disposed = true;
        }
    }
}  

}

and now a derived class:

public class MyDerivedHelper : MyHelper
{
    private bool disposed;

    protected override void Dispose(bool isDisposing)
    {
        if (!this.disposed)
        {
            if (isDisposing)
            {
                //cleanup managed resources here, you may also chain it to a private method in this class
            }

            //cleanup unmanaged resources, you may chain it to a private method in this class

            this.disposed = true;
        }

        //call the base class's dispose
        base.Dispose(isDisposing);
    }
}

Patrick

Patrick Peters
Wed, Jun 24, 2009

Well A short followup. Bill Wagners book also influenced my Dispose Pattern. I own it and read it with joy. The inheritance principle is the same I just make it explicit where to free managed and unmanaged code, which I think is better.

In my pattern I try to make the best pattern possible for all cases. I could make 10 patterns for 10 different cases instead, but that was not the point. You are right about not everything being multithreaded, well then just remove the lock in those cases :O) But it could become multithreaded in the future. It would be the same as not using Transactions if you have only Database reads. You never know what the future will bring.

You are probably right about the Finalizer performance thing. But still the alternative is that your unmanaged resources have a chance of not being cleaned up. If you are correct and it has a serious impact, the best choice would be to remove the destructor when having no unmanaged resources. Would be nice to test this some day.

Jesper
Wed, Jun 24, 2009

Although I had similar thoughts in templatizing out the Dispose(bool) method, the problem is that implementing it like this enforces that managed resources from the entire class hierarchy should always be released in their entirety before any unmanaged resources anywhere along the hierarchy chain. Although somewhat rare, this may not be the case for derived classes. I only bring this up because you’re trying for a “best practice” approach…

As for the locking issue, this should really be done only on classes that are synchronized between threads. The overhead of locking is somewhat small, but can quickly add up when using a lot of small classes. This is more of a nit, but again, you are trying for a “best practice” type of implementation… ;-)

The other issue, of course, is that you should point out that subclasses which override the FreeResources() methods should call base.FreeResources() after doing their work but before returning, to make sure that base classes get a chance to clean their resources up too.

Anonymous Coward
Fri, Sep 18, 2009

Hi Anonymous C :O)

I could consider switching the freeing of managed and unmanaged resources, but on the other hand the freeing of managed resources is really, about freeing unmanaged resources. So in the pattern I have described unmanaged resources will be freed in refered classes first, and in the “top” class last. If I switch the statements it will be the other way around. Will any of these be fool proof ? I guess I will have to give it some thought.

About the locking comment. One size fits all is not always good that is true. So you could remove the lock statement when you are sure it is not needed, or rather when performance demands it. I mean when can you really be sure that your class will never at some point in the future be part of a multithreaded scenario :O)

You are right I should have pointed out that one should call base.Free…() last in derived classes.

Jesper
Sat, Sep 19, 2009

Comments closed.