Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

How to better implement .NET IDisposable classes?

Forgive me in advance if this question is a little too open-ended, but I've seen similar language discussion posts here so I figured I'd take the plunge.

Anyway, I have read several MSDN help pages and various other blogs on the subject of properly implementing IDisposable classes. I feel like I understand things pretty well, but I have to wonder if there's a flaw in the suggested class structure:

public class DisposableBase : IDisposable
{
    private bool mDisposed;

    ~DisposableBase()
    {
        Dispose(false);
    }

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

    protected virtual void Dispose(bool disposing)
    {
        if (!mDisposed)
        {
            if (disposing)
            {
                // Dispose managed resources
                mManagedObject.Dispose();
            }

            // Dispose unmanaged resources
            CloseHandle(mUnmanagedHandle);
            mUnmanagedHandle = IntPtr.Zero;

            mDisposed = true;
        }
    }
}

Anytime the above is supposed to serve as a base class, you rely on the implementer of the subclass to properly override the Dispose(bool) method where necessary. In short, derived classes must ensure they invoke the base Dispose(bool) method from within their overridden version. If not, the base class' unmanaged resources may never get freed, defeating the primary purpose of the IDisposable interface.

We all know the benefits of virtual methods, but it seems like in this case their design falls short. In fact, I think this particular shortcoming of virtual methods manifests itself frequently when trying to design visual components and similar base/derived class structures.

Consider the following change, using a protected event rather than a protected virtual method:

public class DisposeEventArgs : EventArgs
{
    public bool Disposing { get; protected set; }

    public DisposeEventArgs(bool disposing)
    {
        Disposing = disposing;
    }
}

public class DisposableBase : IDisposable
{
    private bool mDisposed;

    protected event EventHandler<DisposeEventArgs> Disposing;

    ~DisposableBase()
    {
        Dispose(false);
    }

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

    // This method is now private rather than protected virtual
    private void Dispose(bool disposing)
    {
        if (!mDisposed)
        {
            // Allow subclasses to react to disposing event
            AtDisposing(new DisposeEventArgs(disposing));

            if (disposing)
            {
                // Dispose managed resources
                mManagedObject.Dispose();
            }

            // Dispose unmanaged resources
            CloseHandle(mUnmanagedHandle);
            mUnmanagedHandle = IntPtr.Zero;

            mDisposed = true;
        }
    }

    private void AtDisposing(DisposeEventArgs args)
    {
        try
        {
            EventHandler<DisposeEventArgs> handler = Disposing;
            if (handler != null) handler(this, args);
        }
        catch
        {
        }
    }
}

With this design, the base class' Dispose(bool) method will always be called, regardless of whether subclasses subscribe to the Disposing event or not. The biggest flaw that I can see with this revised setup is that there is no predetermined order for when event listeners are called. This could be problematic if there are multiple levels of inheritance, e.g. SubclassA's listener might be triggered before its child SubclassB's listener. Is this flaw serious enough to invalidate my revised design?

This design dilemma makes me wish there were some sort of modifier for methods that was similar to virtual but which would ensure that the base class' method was always called, even if a subclass overrode that function. If there's a better way to achieve this, I would greatly appreciate your suggestions.

like image 514
Jeremy Avatar asked Dec 27 '22 12:12

Jeremy


2 Answers

You're using an event here when really you want to use an inheritance mechanism like virtual. For scenarios like this where I want to ensure my implementation is always called but want to allow for base class customization I use the following pattern

private void Dispose(bool disposing)
  if (mDisposed) { 
    return;
  }

  if (disposing) {
    mManagedObject.Dispose();
  }

  // Dispose unmanaged resources
  CloseHandle(mUnmanagedHandle);
  mUnmanagedHandle = IntPtr.Zero;
  mDisposed = true;

  DisposeCore(disposing);
}

protected virtual void DisposeCore(bool disposing) {
  // Do nothing by default
}

With this pattern I've ensured my base class Dispose implementation will always be called. Derived classes can't stop me by simply forgetting to call a base method. They can still opt into the dispose pattern by overriding DisposeCore but they can't break the base class contract.

like image 81
JaredPar Avatar answered Jan 13 '23 19:01

JaredPar


The derived class can simply re-implement IDisposable and thus prevent your dispose method from being called, so you can't ensure that either.

Personally I wouldn't use either pattern. I prefer building on SafeHandle and similar mechanisms, instead of implementing finalizers myself.

like image 37
CodesInChaos Avatar answered Jan 13 '23 20:01

CodesInChaos