36

I have a parent and child class that both need to implement IDisposable. Where should virtual (and base.Dispose()?) calls come into play? When I just override the Dispose(bool disposing) call, it feels really strange stating that I implement IDisposable without having an explicit Dispose() function (just utilizing the inherited one), but having everything else.

What I had been doing (trivialized quite a bit):

internal class FooBase : IDisposable
{
    Socket baseSocket;

    private void SendNormalShutdown() { }

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

    private bool _disposed = false;
    protected virtual void Dispose(bool disposing)
    {
        if (!_disposed)
        {
            if (disposing)
            {
                SendNormalShutdown();
            }
            baseSocket.Close();
        }
    }

    ~FooBase()
    {
        Dispose(false);
    }
}

internal class Foo : FooBase, IDisposable
{
    Socket extraSocket;

    private bool _disposed = false;
    protected override void Dispose(bool disposing)
    {
        if (!_disposed)
        {
            extraSocket.Close();
        }
        base.Dispose(disposing);
    }

    ~Foo()
    {
        Dispose(false);
    }

}
Tanzelax
  • 5,406
  • 2
  • 25
  • 28
  • 1
    [Msdn](https://msdn.microsoft.com/en-us/library/system.idisposable(v=vs.110).aspx0) has recommendations on how to subclass objects implementing IDisposable. – PeterM Aug 18 '16 at 13:52

5 Answers5

33

When I just override the Dispose(bool disposing) call, it feels really strange stating that I implement IDisposable without having an explicit Dispose() function (just utilizing the inherited one), but having everything else.

This is something you shouldn't be concerned with.

When you subclass an IDisposable class, all of the "Dispose pattern" plumbing is already being handled for you by the base class. You really should do nothing but override the protected Dispose(bool) method, and track whether you've been disposed already (to properly raise ObjectDisposedException.)

For details, see my blog post on Subclassing from an IDisposable class.


Also, often, it's a good idea to consider encapsulating the IDisposable class instead of subclassing it. There are times when subclassing an IDisposable class is appropriate, but they are somewhat rare. Encapsulation is often a better alternative.

Reed Copsey
  • 554,122
  • 78
  • 1,158
  • 1,373
  • Ah, I guess my worries were unfounded and I was close to doing it right (I wonder where I picked up the idea of re-implementing the finalizer from... oh well). And yeah, inheritance in general is something I find myself using less and less. – Tanzelax Mar 22 '10 at 23:31
  • Reed your blog is brilliant... but note the count of your answer here - 7 (1 is mine :)).... people cannot appreaciate this enough.. shame. – Boppity Bop May 25 '12 at 13:13
5

I always turn to Joe Duffy's very in-depth study on this pattern. For me, his version is Gospel.

http://joeduffyblog.com/2005/04/08/dg-update-dispose-finalization-and-resource-management/

The first thing to remember is that a finalizer is not needed most of the time. It's for clearing up unmanaged resources where you are directly holding native resources, i.e. only resources that do not have their own finalizer.

Here's an example for a base-class subclass pair.

// Base class

    #region IDisposable Members

    private bool _isDisposed;

    public void Dispose()
    {
        this.Dispose(true);
        // GC.SuppressFinalize(this); // Call after Dispose; only use if there is a finalizer.
    }

    protected virtual void Dispose(bool isDisposing)
    {
        if (!_isDisposed)
        {
            if (isDisposing)
            {
                // Clear down managed resources.

                if (this.Database != null)
                    this.Database.Dispose();
            }

            _isDisposed = true;
        }
    }

    #endregion


// Subclass

    #region IDisposable Members

    private bool _isDisposed;

    protected override void Dispose(bool isDisposing)
    {
        if (!_isDisposed)
        {
            if (isDisposing)
            {
                // Clear down managed resources.

                if (this.Resource != null)
                    this.Resource.Dispose();
            }

            _isDisposed = true;
        }

        base.Dispose(isDisposing);
    }

    #endregion

Note that the subclass has its own _isDisposed member. Also note null-checking on resources since you don't want any exceptions in these blocks.

Luke

Luke Puplett
  • 42,091
  • 47
  • 181
  • 266
5

Why complicate things when you don't need to?

Since you don't encapsulate any unmanaged resources, you don't need all that mucking about with finalization. And, your classes are internal, which suggests that you control the inheritance hierarchy within your own assembly.

So, the straighforward approach would be:

internal class FooBase : IDisposable 
{ 
  Socket baseSocket; 

  private void SendNormalShutdown() 
  { 
    // ...
  } 

  private bool _disposed = false; 

  public virtual void Dispose() 
  { 
    if (!_disposed)
    { 
      SendNormalShutdown(); 
      baseSocket.Close(); 
      _disposed = true;
    } 
  } 
} 

internal class Foo : FooBase
{ 
  Socket extraSocket; 

  private bool _disposed = false; 

  public override void Dispose()
  { 
    if (!_disposed)
    { 
      extraSocket.Close(); 
      _disposed = true;
    } 

    base.Dispose(); 
  } 
} 

Even when you do have unmanaged resources, I'd say you're much better off encapsulating them in their own disposable class and using them like you'd use any other disposable; as straighforward as the code above.

Luke Puplett
  • 42,091
  • 47
  • 181
  • 266
Jordão
  • 55,340
  • 13
  • 112
  • 144
  • 1
    Can you think of any case where a subclass should have a cleanup finalizer when a parent class doesn't (aside from the trivial case of inheriting from Object)? I can understand adding a finalizer whose job is to complain of improperly-abandoned objects, but IMHO anything requiring cleanup should be in its own finalizable class; I can think of *NO* exceptions. – supercat Apr 16 '11 at 19:31
  • @supercat: I completely [agree](http://codecrafter.blogspot.com/2010/01/revisiting-idisposable.html). – Jordão Apr 17 '11 at 01:15
  • You should prevent them from overriding the public Dispose method and instead override the protected method. See: https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/implementing-dispose#implementing-the-dispose-pattern-for-a-derived-class – David Anderson Nov 20 '18 at 14:03
  • Hey @DavidAnderson, there's no protected method, as I'm not following the Microsoft pattern for disposables _on purpose_. Take a look at my approach [here](http://codecrafter.blogspot.com/2010/01/better-idisposable-pattern.html) and [here](http://codecrafter.blogspot.com/2010/01/revisiting-idisposable.html). – Jordão Nov 20 '18 at 16:05
4

The idea of this pattern is that you override the virtual Dispose method, calling base.Dispose if necessary. The base class takes care of the rest, calling the virtual Dispose method (and hence the correct implementation). The subclass should not need to also implement IDisposable (it is IDisposable via inheritance)

spender
  • 117,338
  • 33
  • 229
  • 351
  • 1
    I suppose I could just take out the explicit `: IDisposable`, since it's already inheriting that interface. I think I do like explicitly stating that "this class has some special disposing that happens". Thanks for the input, though. :) – Tanzelax Mar 22 '10 at 23:36
2

The child class should override the virtual Dispose, do any disposing specific to the subclass, and call the superclass' Dispose, which in turn will do its own work.

EDIT: http://davybrion.com/blog/2008/06/disposing-of-the-idisposable-implementation/ is the pattern I follow in such cases. Not the 'Disposable' class specifically, but the inheritance and overrides.

Grant Palin
  • 4,546
  • 3
  • 36
  • 55
  • 1
    I'm not sure I like the idea of adding even more virtual/abstract methods to the already-bloated IDisposable pattern... – Tanzelax Mar 22 '10 at 23:36
  • @Tanzelax To each their own, I suppose. The provided code has been useful to me where I have a hierarchy of classes that need to be disposable. Most of the setup is in the top-level class, and minimal extra code is needed per subclass. – Grant Palin Mar 23 '10 at 00:51