3

I'm being asked to implement IDisposable on objects that are 100% managed resources, which contain no streams, and no large resources.

I understand the importance of properly disposing of large resources and unmanaged resources, but what about the other side of the equation?

For an object that doesn't benefit from implementing IDisposable (small fully managed objects), what negative impact can GC.SuppressFinalize have, if any?

4
  • Looks like premature optimization. Are you in a position to ask the person who asked you to do that the results he got from profiling and benchmarking which show that adding IDisposable in this case improves memory footprint? Jul 26, 2014 at 17:29
  • @MainMa No but that didn't stop me from asking anyways. The result was a 5 minute response of "its just the way we've always done it, you do it too". Jul 26, 2014 at 17:33
  • Well, it is possible to implement IDisposable without writing a destructor/finalizer. I don't know if the person who asked you to implement IDisposable meant you should make a destructor as well. Of course, if the class has no destructor, there is no point in GC.SuppressFinalize. If the class does have a destructor, be sure to say GC.SuppressFinalize(this) when your Dispose method has run. Otherwise the instance will not be garbage-collected easily, it will be queued on a finalizer queue and only be collected in a generation-2 collect. Jul 27, 2014 at 19:59
  • 1
    @JeppeStigNielsen The question states that the objects are dealing with entirely managed resources, so there's no need to have a finalizer. If one is there, we know it's wrong.
    – Servy
    Oct 4, 2016 at 16:29

2 Answers 2

5

The only goal of GC.SuppressFinalize is:

to prevent the finalizer from releasing unmanaged resources that have already been freed by the IDisposable.Dispose implementation.

Source: MSDN

Once you disposed the object, you should indeed call GC.SuppressFinalize(this);, like shown in an answer to the question "When should I use GC.SuppressFinalize()?". Code analysis' CA1816 rule is also useful as well.

Now, implementing IDisposable on objects which don't have unmanaged resources looks quite weird and questionable. Since the only reason the person gave you was: "its just the way we've always done it, you do it too", instead of providing profiling/benchmarking data which shows that adding IDisposable will improve something in a specific case, there are no actual reasons to do it.

Would it cause performance problems? Difficult to say: it would depend on particular case.

Would it cause any other problems? Of course. Not counting the fact that IDisposable is painful to implement, if this practice is used too much, the code becomes unreadable and impossible to maintain:

public int ComputePrice(Rebate rebate)
{
    using (var currentCurrency = this.User.FindCurrency())
    {
        using (var priceWithoutRebate = this.GetPurePrice())
        {
            using (var canApplyRebate = rebate.CanApplyTo(this))
            {
                if (!canApplyRebate)
                {
                    return priceWithoutRebate;
                }

                using (var priceWithRebate = priceWithoutRebate.Apply(rebate))
                {
                    return priceWithRebate;
                }
            }
        }
    }
}
2
  • Well I believe the story is that a long time ago, they had some connection/stream leaking issues, probably due to leaky connection/command ADO.NET code, and this caused massive resource leaks. They knew that implementing IDisposable on everything would solve the problem, so they did, and it worked. And so now they just implement it on most objects. Jul 26, 2014 at 17:44
  • 2
    @AndrewHoffman: it seems like a plausible explanation. But it still doesn't justify the current practice. Jul 26, 2014 at 17:49
2

You do not have to use the heavy-weight ("painful") Dispose+Finalizer pattern on a IDisposable class, if the class is sealed, and the class has no unmanaged resources.

In that situation, you can use a subset of the Dispose+Finalizer pattern, which just has public Dispose(), has no finalizer, and the class is sealed.

The reason for sealing the class is to avoid the problems of "What if a child class has unmanaged resources?" and "What if a child class in the future will have unmanaged resources?"

Joe Duffy, formerly of Microsoft, has a lengthy (30+ page) article that discusses how to correctly implement IDisposable, and all the permutation subset implementations.

Contributors to the article include Herb Sutter, Brian Grunkemeyer, Jeff Richter, and other C# luminaries. The article is much more in-depth than what is presented on MSDN.

Alas, Joe Duffy's new blog engine has not done a good job of preserving the original article from 2005, so it looks somewhat munged. Here is the link: http://joeduffyblog.com/2005/04/08/dg-update-dispose-finalization-and-resource-management/

I wish I could attach my MS-Word document that has the entirety of Joe Duffy's article in a much better format. :-(

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.