Will this be a valid base class for IDisposable

Question

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
    }
}

2. Not disposed object got reported

3. 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?

Cuthburt

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. – Baxter

Makes more sence to specify ConditionalAttribute on the finalizer itself, since classes with a finalizer defined imply some overhead. – Nebulosity

@ 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? – Cuthburt

@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. – Cuthburt

IMHO this could be good, if we were able to use conditional error directive, to give developer a compilation error. So when the developer uses a class he wouldn't be able to build a project unless he had propertly implemented dispose in code. What you suggest is throwing exception at a runtime if object wasn't disposed and is about to be finallized by GC. But what if GC isn't ever going to finallize it? – Danadanae

@Danadanae The purpose of the guideline is to prevent everybody from implementing IDispose on his/her own. In a big team, there will always be somebody who get it wrong. Finalizer is not guaranteed to run. So the error reporting benefit is not 100% guaranteed. But it is still better than nothing. – Cuthburt

What I meant is that this would have been cool if we could do 2 things: throw compilation error if developer doesn't call dispose on an object and forbid overriding finalize method. – Danadanae

Btw this is achievable through using mono.cecil and custom build tasks. Thought its hard – Danadanae

@Danadanae Good to know. It is also achievable with FxCop – Cuthburt

I would recomend you to read the chapter 5 of the book MCSD Certification Toolkit (Exam70-483) where it covers with a good material this topic of how and when implementing IDisposable interface. I can give you an answer but right now I'm kind of busy here in my work. I will track this post to answer it later. – Jacaranda

#32354886 this will be interesting to you. You almost never need the dispose pattern at all thanks to SafeHandle. – Coequal

That Dispose() = 0 betrays that you're coming from C++. In C# we don't really deal with unmanaged resources anymore, so this is all academic. Just never write a ~destructor and you're good. Look up the SafeHandle class. – Khanate

@HenkHolterman I guess one can never hide his tail :). Corrected. – Cuthburt

Answer 1

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.

Mazurka

Answer 2

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.

Vicious

Won't GC.SuppressFinalize() remove it from the queue? – Cuthburt

It will, and that is an extra work runtime needs to perform. Plus work needed to add it into the queue. – Vicious

I could be mistaken, and I can't find the info now, but I believe GC.SuppressFinalize() doesn't need to do anything more than to mark a bit flag in the object's header to indicate it doesn't need to be finalized. The runtime performance hit of this is negligible - the object is never added to the finalizer queue, as during the collection sweep the flag is checked on every object. – Carpogonium