Recently I have been reviewing some of the production C# code where I noticed that the programmer was calling Dispose() method in the class constructor. He was doing that for a reason, because that particular class was wrapping on several unmanaged resources. Resources were initialized right in the constructor one after another in a try/catch block. The catch invoked Dispose() to make sure that everything already initialized would go away should an exception be thrown down the road. Even though the approach seems viable, this practice is very dangerous as it usually leads to very unpleasant side effects.

Let us see what can happen is such situation. Consider a base class that invokes Dispose() in its constructor (naturally, it implements IDisposable interface as well):

public class BaseClassWithDispose : IDisposable
{
  protected AdvancedBaseClass ()
  {
    // do not do this!
    try { ... } catch { Dispose () ; throw ; }
  }

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

  protected virtual void Dispose (bool disposing)
  {
    // usual dispose pattern implementation
  }
}

Except for the Dispose() invocation this class looks ok. Now, let’s extend this class by a class that also wraps an unmanaged resource.

public class UnmanagedResourceWrapper : BaseClassWithDispose
{
  private Resource m_unmanagedResource ;
  private bool m_disposed = false ;

  public UnmanagedResourceWrapper ()
  {
    m_unmanagedResource = ... ;
  }

  protected override void Dispose (bool disposing)
  {
    if (!m_disposed)
    {
      if (m_unmanagedResource != null)
        m_unmanagedResource.Release () ;
    }

    m_disposed = true ;
    base.Dispose (disposing) ;
  }
}

Note, that this class allocates the resource in the constructor (we omit try/catch sections for the sake of presentation).

Nothing special so far. But, wait… let’s take a closer look at what happens when Dispose() gets called in the base class. Because of the fact that Dispose(bool) method is virtual the UnmanagedResourceWrapper.Dispose(bool) will be called prior to its constructor invocation. That means that the instance is going to be disposed before it even gets a chance to proper construct itself. Should this be a purely managed instance you wouldn’t even notice, but we are wrapping on the unmanaged resource here. The constructor would eventually be called (disposing makes no difference here) and the unmanaged resource gets allocated. However, as you may have guessed already, it would never be released (m_disposed is already set to true at this point). Voilà, we get a resource leak.

Morale of the story?

• do not call Dispose() in constructors,
• do not initialize (unmanaged) resources in constructors, and if you really need to, make sure to release them manually and implement a proper Dispose pattern to handle this situation.