Disposing of a collection and then it's contents

Question

I have an object, which contains a collection of other objects. I want to dispose of them all by calling the dispose method on the base object. The dispose method of the collection will clear the collection, but it does not dispose of the individual objects it contains. So I need my base object to loop through and dispose of each child object. The last detail is that I want to make sure that the collection is disposed of before the individual objects it contains.

I am thinking I can get this behavior by using Dispatcher.BeginInvoke and a low priority. First passing it the dispose call for the collection and then looping through and disposing each individual item. (pseudo) code looking something like this:

class Foo
{
    FooCollection Children;

    void Dispose()
    {
        //unsubscribe from data model events

        CurrentDispatcher.BeginInvoke(Children.Dispose, ApplicationIdle, null);
        foreach (Foo Child in Children)
        {
            CurrentDispatcher.BeginInvoke(Child.Dispose, ApplicationIdle, null);
        }
    }
}
class FooCollection
{
    void Dispose()
    {
        //unsubscribe from data model events

        this.Clear();
    }
}

Am I on the right track? Or am I just setting myself up with a race condition? i.e. is it possible that the Dispatcher could get around to calling dispose on the collection before the foreach finishes queing up the dispose calls for the children? Is there a better way to get the desired behavior?

It is probably worth noting that each of these classes exposes events, and hooks handlers into other objects. My primary goal is to make sure all of the handlers get cleaned up gracefully when dispose is called.

EDIT (8/24): The instances of these classes subscribe to events from objects which generally persist for the life of the application. Because of this, the persistent object maintains a reference to each of these instances. I am trying to make sure that each instance unsubscribes from the persistent object's events when the instance is no longer needed.

EDIT 2 (8/30): This is part of a view model, which represents a data hierarchy retrieved from a web service. The cost of retrieving the data from the web service is high, so the actual data objects returned are cached, generally for the life of the application. The data objects implement ICollectionChanged and IPropertyChanged, the view model objects in question subscribe to these events. The nature of the application is such that user actions will likely cause the view model to be discarded and recreated several times during a session. The nature of the data is such that there will commonly be several thousand nodes in the hierarchy which have to be represented in the model. The primary concerns, as they relate to this question, are:

1) Making sure that the discarded view model unsubscribes from the events of the underlying data model

2) Making sure it is done in such a way that looping through the full hierarchy of the discarded view model does not noticeably impact the user in working with the new instance of the view model.

I have updated the code example to more accurately represent the situation. So the question remains. Will this code give me the expected result in that the collection, and then all of its children, will be queued up for disposal before any object actually gets disposed, and then some time later the thread will start disposing them when it has idle time? Or is it going to create a race condition where the collection could possibly be disposed of before I have finished looping through it?

Most posters have focused on the fact that the collection gets cleared, in all honesty clearing the collection is probably unnecessary, but it is a bit of a habit, and I consider it to be good housekeeping.

Why not have BarCollection dispose of each item in it when dispose is called? Also as nick says why are you implementing IDisposable does bar contain references to unmanaged resources? — Ben Robinson, Aug 24 '11 at 19:33
@Nick They are managed but there is high potential that many instances will be created, used for a short time, and then discarded. Because these objects hook event handlers into objects which are more persistent, there is a high probability that an instance will be kept alive long after it should no longer be in use. Hence the reason for trying to have `Dispose` clean up the event handlers. — Rozwel, Aug 24 '11 at 19:37
@Ben Having the collection dispose of the contents is not an easy option, there are other places where the collection class would need to be disposed of without disposing of its contents, i.e. there are other references to the Bar objects which are still valid. — Rozwel, Aug 24 '11 at 19:41
@Rozwel Aren't you trying to second guess the garbage collector,what makes you think that there is anything you can do in the dispose method of bar that will it make more likely to be garbage collected or that the fact that it has events wired up to methods that persist will make it less likely to be garbage collected. I strongly suspect that what you are doing is a big fat waste of time. You should implement IDisposable if you class has members that Implement IDisposable and you want to make sure they are dispose or your class uses unmanaged resources that need manually cleaning up. — Ben Robinson, Aug 24 '11 at 19:43
@Rozwel If you don't want the dispose method of BarCollection to dispose of its contents what do you want it to do? Simply clearing its contents is pointless. — Ben Robinson, Aug 24 '11 at 19:46
@Ben The dispose method of all three classes is primarily intended to unsubscribe from events. — Rozwel, Aug 30 '11 at 13:08

score 1 · Answer 1 · edited May 23 '17 at 11:43

Check this link out, it has a nice explanation of garbage collection with regards to event handlers: What best practices for cleaning up event handler references?

It really depends on how your events line up. If you have objects that receive events from other, more persistent objects, they will stick around to receive those events until the event sources are GCed. This indicates that your dispose method should probably clear the events that you are hooked up to (with the -= operator) if you really need to reclaim this memory (how big are these objects? what's the system you are running this on?) You'll have to call this Dispose method yourself when you're done with the objects...

Clearing the collection is not necessary - just make sure all the references to any objects that you are done with are set to null. When no references exist and no event handlers must be maintained, the GC will collect the object.

I also wouldn't worry about the dispatcher.. unsubscribing from events shouldn't pose a performance issue.

You don't choose when to dispose managed objects - the GC does. You can't dispose a collection before you dispose its children.... because you can't dispose a collection at all! You can only prepare a managed object for garbage collection, and the way to do that is

(1) Clear event handlers and

(2) Get rid of all the references to the objects you are done with.

The GC will take care of the rest.

Thank you for the link, it confirms that I need to clean up my events if I don't want the old view model hanging around after it is no longer needed. — Rozwel, Aug 30 '11 at 15:17

score 0 · Answer 2 · answered Aug 24 '11 at 19:30

0

You won't be able to guarantee the sequence the dispatcher calls dispose. Given that, it'd be better in my opinion to dispose of the individual items in the collection and then call dispose on the collection.

If the collection is cleared prior to the foreach loop completing you could end up with an exception.

answered Aug 24 '11 at 19:30

Hasanain

925
8
16

From http://msdn.microsoft.com/en-us/library/ms591206.aspx `If multiple BeginInvoke calls are made at the same DispatcherPriority, they will be executed in the order the calls were made.` So the question is can the dispatcher drop down and execute something at that priority level before the loop completes. – Rozwel Aug 24 '11 at 19:56

score 0 · Answer 3 · answered Aug 24 '11 at 19:32

0

I would suggest you a bit another design. Please note, it's not final code, just to show the idea.

class Bar:IDisposable
{
    void Dispose()
    {
    //do some logic
    }
}
class BarCollection:List<Bar>,IDisposable
{
    void Dispose()
    {
        foreach(Bar bar in this){
            bar.Dispose();
        }
    }
}

class Foo
{
    BarCollection Children;

    void Dispose()
    {
        CurrentDispatcher.BeginInvoke(Children.Dispose, ApplicationIdle, null);
    }
}

answered Aug 24 '11 at 19:32

Pashec

23,199
3
26
26

I initially started with this approach, but then realized there were cases where the same collection class was used for a different purpose and having it dispose of its contents in those cases would cause problems. The contents of the collection should only be disposed when it was created as a member of the foo class. – Rozwel Aug 30 '11 at 14:09

Disposing of a collection and then it's contents

3 Answers3