44

I need some advice on the implementation of the Dispose method.

In our application the user designs their own UI. I have a preview window that shows what the UI is going to look like. All object drawn in this UI ultimately derive from a common base class ScreenObject. My preview manager contain a single object reference to a ScreenGrid which is the grid object for the entire preview area.

Question #1

Some of my derived screen classes hold onto unmanaged resources, such as a database connection, bitmap image and a WebBrowser control. These classes need to dispose of these objects. I created a virtual Dispose method in the base ScreenObject base class and then implemented an override Dispose method in each of the derived classes that hold onto unmanaged resources. However, right now I just created a method called Dispose, I am not implementing IDisposable. Should I implement IDisposable? If so how do I implement it?

  • Just on the derived classes that have unmanaged resources
  • The base class and derived classes that have unmanaged resources OR
  • The base class and all derived classes including those that do not have unmanaged resources

Is it wrong to put a virtual Dispose method in a base class that doesn't have unmanaged resources so that you can take advantage of polymorphism?

Question #2

In reading about the Dispose method and the IDisposable interface Microsoft states that the disposing object should only call the Dispose method for its parent. The parent will call it for its parent and so on. To me this seems backwards. I may want to dispose of a child but keep its parent around.

I would think it should be the other way around, an object being disposed should dispose of its children. The children should then dispose of their children and so on.

Am I wrong here or am I missing something?

10
  • 4
    To your second question. Maybe you misunderstood something. Microsoft only says that the class should call its parent class Dispose method, which means the method of the base class. I agree that the class only have to dispose its children and at the end its base.
    – MatthiasG
    Commented Sep 20, 2011 at 12:28
  • Do you hold unmanaged resources in your class? Or do you just own a class which in turn owns the unmanaged resources? I hope it's the second, else you will have to deal with fun stuff such as critical finalization. Commented Sep 20, 2011 at 12:30
  • 3
    Question #1: You should implement IDisposable so that you (and others) can then use the using statement in C#. Having a Dispose method without implementing IDisposable doesn't really make sense, you could just call it MyCleanUp as well.
    – Marc
    Commented Sep 20, 2011 at 12:32
  • Your class ScreenObject could also have an abstract method MyCleanUp to force its implementation. With good documentation it should be clear what implementators have to do.
    – Marc
    Commented Sep 20, 2011 at 12:35
  • 2
    This could well be the 100th duplicate of this question. Did you bother even looking at the list of similar questions? Commented Sep 20, 2011 at 12:36

3 Answers 3

47

Question 1: Implement IDisposable as well, using the following pattern:

public class MyClass : IDisposable
{
    bool disposed;

    protected virtual void Dispose(bool disposing)
    {
        if (!disposed)
        {
            if (disposing)
            {
                //dispose managed resources
            }
        }
        //dispose unmanaged resources
        disposed = true;
    }

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

Question 2: What Microsoft means is that a derived class calls dispose on it's parent class. The owner of the instance only calls Dispose on the most derived type.

An (shortened) example:

class Parent : IDisposable 
{
    protected virtual void Dispose(bool disposing)
    {
        if (!disposed)
        {
            if (disposing)
            {
                //dispose managed resources
            }
        }
        //dispose unmanaged resources
        disposed = true;
    }

}
class Child : Parent, IDisposable 
{ 
    protected override void Dispose(bool disposing)
    {
        if (!disposed)
        {
            if (disposing)
            {
                //dispose managed resources
            }
            base.Dispose(disposing);
        }
        //dispose unmanaged resources
        disposed = true;
    }

}
class Owner:IDisposable
{
    Child child = new Child();
    protected virtual void Dispose(bool disposing)
    {
        if (!disposed)
        {
            if (disposing)
            {
                if(child!=null)
                {
                    child.Dispose();
                }
            }
        }
        //dispose unmanaged ressources
        disposed = true;
    }
}

The owner only calls Dispose on the Child, but not on the Parent. The Child is responsible for calling Dispose on the Parent.

9
  • FxCop rules gives a sample implementation of this case: msdn.microsoft.com/en-gb/library/ms182330(VS.80).aspx
    – Ucodia
    Commented Sep 20, 2011 at 12:41
  • 1
    No, using the disposing pattern is wrong 99.9% of the time. Only used if the class has a finalizer, it shouldn't have one. Wrap unmanaged resources in their own class, like one of the SafeHandle derived classes. Commented Sep 20, 2011 at 12:45
  • In your second answer, where would the declaration of the non virtual Dispose method with no parameters go? My thought is that you could put this in just the Parent class, or would both Parent and Child implement it?
    – WPFNewbie
    Commented Sep 20, 2011 at 12:51
  • 1
    There is an error in that answer. Dispose method shouldn't be calling base.Dispose() - which is causing stack overflow. Instead it should call protected base object method, possibly base.Dispose(true) Commented Sep 17, 2013 at 8:36
  • 1
    I suggest moving base.Dispose(disposing) in the Child class outside of the if (disposing) logical block as the Parent class contains unmanaged resources and that block is reserved for freeing managed resources. If Dispose(false) is called on the Child then given the current implementation - the unmanaged resources that the Parent owns will not be freed.
    – Derek W
    Commented Mar 9, 2015 at 17:06
8

Question 1:

Based on the types of objects that you list (i.e. Database, WebBrowser, Bitmap, etc.) these are only managed resources as far as .NET is concerned. Thus, you should implement IDisposable on any class that has disposable types as members. If they are locally declared instances, you just call 'using()' on them. While these instances you mention do have unmanaged resources under them, this is abstracted away from you by .NET thru the types you are using. Since you are only using managed types, you should implement IDisposable but without a finalizer. You only need to implement a finalizer if you truly have unmanaged resources as class members.

Question 2:

It seems that you are confusing inheritance (is a) with aggregation/containment (has a). For example, if "Container" contains a disposable resource as a class member, it is called aggregation/containment. So, calling base.Dispose() in the IDisposable implementation of Container has nothing to do with disposing of the disposable resource inside of Container. You should remember that if a class derives from Container, say "DerivedContainer", that it is an instance of Container albeit with additional members and/or functionality. So any instance of "DerivedContainer" has all of the members that its base class "Container" does. If you never called base.Dispose(), the disposable resource in "Container" would not be released properly (it would actually by the GC, but it's bad practice for many reasons to just 'let .NET take care of it') - please refer to my posted answer at Is it bad practice to depend on the .NET automated garbage collector?.

If you didn't call the base class Dispose(), you'd end up with a partially disposed object (disposed in the derived class but not in the base class) - a very bad scenario. So it is very important to call the base class Dispose().

I have a best practices pattern I've developed (with lots of experience and debugging memory dumps) written on my blog as an example. It shows how to implement IDisposable on a base class as well as a derived class:

http://dave-black.blogspot.com/2011/03/how-do-you-properly-implement.html

2
  • The question was quite old by the time you responded to it. I took a look at your blog post, though. You seem to repeat the normal Microsoft pattern, defects and all. Among other things, rather than using one flag to provide thread-safe protection against multi-disposal for all derived classes, it requires a separate flag for every derived class and is not thread-safe. Also, since classes with finalizers should avoid holding objects not needed for finalization, derived classes shoudln't hold unmanaged resources if the base class does not.
    – supercat
    Commented Dec 14, 2011 at 0:15
  • I am confused by your comment... I responded only 3 months after the question was asked - surely that is not 'quite old'. My pattern is different from the Microsoft pattern. I have more safety precautions, include examples for disposing of unmanaged objects in the Dispose method, add comments about if you should be using a finalizer, comments about when you might need to implement thread synchronization. It is bad practice to include thread synchronization if you do not need to. Also, please enlighten me on the "defects" to which you refer.
    – Dave Black
    Commented Nov 26, 2012 at 18:26
1

I Implement IDisposable

 class ConnectionConfiguration:IDisposable
{
    private static volatile IConnection _rbMqconnection;
    private static readonly object ConnectionLock = new object();
    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }
    protected virtual void Dispose(bool disposing)
    {
        if (!disposing)
        {
            return;
        }
        if (_rbMqconnection == null)
        {
            return;
        }
        lock (ConnectionLock)
        {
            if (_rbMqconnection == null)
            {
                return;
            }
            _rbMqconnection?.Dispose();//double check
            _rbMqconnection = null;
        }
    }
}
1
  • 3
    can you explain why did you used double checking here. Commented Feb 12, 2019 at 6:11

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