8
votes

IDisposable pattern is expensive to implement. I've counted 17 lines of code before even starting to actually dispose resources.

Eric Lippert recently wrote a blog post bringing up an interesting point: any time a finalizer runs, it is a bug. I think it make perfect sense. If the IDisposable pattern is always followed, Finalizer should always be suppressed. It will never have a chance to run. If we accept that finalizer run is a bug, then does it make sense to have a guideline to force developers to derive from the following abstract class and forbid directly implementing the IDisposable interface.

public abstract class AbstractDisaposableBase: IDisposable
{
    ~AbstractDisaposableBase()
    {
        ReportObjectLeak();
        Dispose(false); 
    }

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

    protected abstract void Dispose(bool disposing);

    [Conditional("DEBUG")]
    private void ReportObjectLeak()
    {
        //Debug.Assert(false, "leaked instance");
        //throw new Exception("leaked instance");
    }
}

The benefits are clear:

  1. Implementing dispose becomes easy and error free like below:


class MyClass1 : DisablableBase
{
    protected override void Dispose(bool disposing)
    {
        //dispose both managed and unmamaged resources as though disposing==true
    }
}
  1. Not disposed object got reported

  2. Disposable pattern is always followed

But, is there any problem with such a guideline?

One possible problem is that all disposable object will have a finalizer defined. But since the finalizer is always suppressed, there should not be any performance penalties.

What are your thoughts?

2
Personally I follow this advice posted here: blog.stephencleary.com/2009/08/…. In my software I don't really have unmanaged resources, so I just implement Dispose without finalizer.Dirk
Makes more sence to specify ConditionalAttribute on the finalizer itself, since classes with a finalizer defined imply some overhead.George Polevoy
@ Dirk The point is that if this guideline is valid, it will make implementing IDisposable easy and error free. There will not be other guidelines necessary any more. Except the "Don't do it unless you need it" one though. B.T.W are you the Dirk I know?Xiaoguo Ge
@GeorgePolevoy But if it's execution is suppressed during dispose then there should not be any performance penalties. I've modified the finalizer to actually call Dispose(false) to make it useful even in release.Xiaoguo Ge
stackoverflow.com/questions/32354386/… this will be interesting to you. You almost never need the dispose pattern at all thanks to SafeHandle.usr

2 Answers

4
votes

does it make sense to have a guideline to force developers to derive from the following abstract class

No, solely for the reason that C# doesn't have multiple inheritance. Interfaces describe behavior, inheritance dictates "is-a". You'll thoroughly limit the object-oriented design of your classes if you enforce this rule.

For example you can't introduce base classes for business objects that are not disposable, where a derived class would be.

0
votes

But since the finalizer is always suppressed, there should not be any performance penalties.

Instances of the AbstractDisaposableBase subclasses will still be participating in the finalization queue management, so there will be a performance impact for this.