Suggestions on how to avoid disposing of an object twice

Question

I have a a list of disposable items that I am adding to collection that already contains a number of disposable items. I wrap the code in a try...finally block so that if an exception is thrown while I am copying the items from the list to the collection all the objects in the list get disposed of correctly:

private static void LoadMenuItems(ApplicationMenu parent)
{
    List<ApplicationMenuItem> items = null;
    try
    {
        items = DataContext.GetMenuItems(parent.Id);
        foreach (var item in items)
        {
            parent.Items.Add(item);
        }
        items = null;
    }
    finally
    {
        if (items != null)
        {
            foreach (var item in items)
            {
                item.Dispose();
            }
        }
    }
}

If an exception occurs after adding a number of the objects to the collection, I'll have a situation where the collection contains some disposed objects. Which could give rise to those disposed objects being disposed of again in the following try...catch block:

try
{
    // Assume that menu.Items contains some items prior
    // to the call to LoadMenuItems.
    LoadMenuItems(menu);
}
catch
{
    // The Dispose() iterates through menu.Items calling 
    // Dispose() on each.
    menu.Dispose();
}

So what I am looking for possible solutions to stop Dispose() being called twice. I have a solution in mind, but thought I would give it to the community to see if there are any alternatives.

H H · Accepted Answer · 2012-01-11T19:10:21.377

5

Which could give rise to those disposed objects being disposed of again

Which should be quite alright. The contract for Dispose() is very specific: it should be safe to call it multiple times.

But another way to get rid of it:

Analyzing your logic I would say the finally part is superfluous, maybe the analyzer thinks so too. You really do solve the same problem twice.

edited Jan 11 '12 at 19:10

answered Jan 11 '12 at 17:11

H H

263,252
30
330
514

I thought it would fine but the code analysis tool in Visual Studio 2010 gives me the warning: "CA2202: Do not dispose objects multiple times" - http://msdn.microsoft.com/en-us/library/ms182334.aspx. I would like to conform! – Ɖiamond ǤeezeƦ Jan 11 '12 at 17:18
Yes, but this warning is very suspect. See [this answer](http://stackoverflow.com/a/3839419/60761) – H H Jan 11 '12 at 17:29

score 2 · Answer 2 · answered Jan 12 '12 at 18:29

Most cases where one might be worried about "accidentally" disposing an object multiple times come about because there is confusion about who "owns" the object in question, and such confusion will likely create other problems in addition to repeated disposal. While one could avoid having the multiple-disposal itself cause problems by having the disposal method use a flag so the second dispose attempt will return harmlessly, doing that without resolving the confusion about IDisposable ownership would not lave the more serious issues unresolved.

The primary scenarios where repeated disposal attempts should not be regarded as indicating broader problems are

Situations where creation of an object fails with a partially-constructed object; while one could probably define policies as to which parts of a partially-constructed object are responsible for cleaning up which other parts, it may be easier to simply have each part that's told to perform an "emergency" cleanup to tell every part it knows about to do likewise if it hasn't already. In most disposal scenarios, confusions regarding object ownership can result in objects being disposed while still in use, but if an object factory fails, that generally implies that no references to the object have been released to anyone who is going to use them.
Situations where disposal of an object which is in use is a legitimate usage scenario with predictable semantics. For example, some data sources have blocking wait methods, and explicitly specify that disposal of the data source while a blocking method is waiting on it will that method to fail without further delay. In some cases, it may well be that yanking the disposable resource out from under the task that's using it is the only way for such a task to become unstuck.

Your scenario seems somewhat like the first one, except that it looks like you'll be disposing of each item after it gets added to parent.Items, which would suggest that you may have muddled issues of object ownership.

Suggestions on how to avoid disposing of an object twice

2 Answers2