iterating through disposable objects

Question

Is the following a safe way of iterating through disposable objects? Or will this result in indisposed objects? etc? What if I used dispose statements instead of the using nests?

public static void Main()
{
   foreach (ChildObject oChild in webApp)
   {
        //On Noes!  Unexpected Error!
   }
}

public static IEnumerable<ChildObject> SafelyGetNextObjInWebApp(WebApplication webApp)
{
    foreach (ParentObject oParent in webApp.Parents)
    {
        using (parent)
        {
            foreach (ChildObject oChild in oParent.Children)
            {
                using (oChild)
                {
                    yield return oChild;
                }
            }
        }
    }
}

You may also consider reviewing this post: http://stackoverflow.com/questions/1539114/yield-return-statement-inside-a-using-block-disposes-before-executing — Steve Guidi, Aug 22 '12 at 17:18

Steve Guidi · Accepted Answer · 2012-08-22T17:20:14.600

4

Your method is not safe unless your caller enumerates through all of the objects returned by SafelyGetNextObjInWebApp(). Consider what happens in the following statement:

ChildObject o = SafelyGetNextObjInWebApp(arg).First();

In this case, exection of SafelyGetNextObjInWebApp() will halt at the yield statement, and never continue. Thus the object will not be disposed of by this call.

If you want to use an iterator to return web-service-created objects one at a time, you should make sure the call is exception safe and impose that the caller of the iterator call dispose. To illustrate:

public IEnumerable<ParentObject> GetParents(WebApplication webApp)
{
    // assumes webApp.Parents uses deferred execution.
    return webApp.Parents;
}

public void ProcessParent(WebApplication webApp)
{
    foreach (ParentObject p in GetParents())
    {
        // Assumes p.Dipsose() calls ChildObject.Dispose() for all p.ChildObjects.
        using(p)
        {
            foreach (ChildObject o in p.ChildObjects)
            {
                // do something with o
            }
        }
    }
}

}

edited Aug 22 '12 at 17:20

answered Aug 22 '12 at 17:08

Steve Guidi

19,700
9
74
90

Do you have any suggestions on how I could write this? Do I have to write 2 ienumerables, one for ParentObject and one for ChildObject, and expect the caller to call both and dispose properly? This seems just as messy as leaving the entire statement as the first few lines of the main method. – ep1033 Aug 22 '12 at 17:12
You can also make ParentObject.Dispose() call Dispose() on all the ChildObject references, thus eliminating the section using block. However, you should enforce that the caller invoke Dispose() on each ParentObject retrieved by the web app. (editted to illustrate) – Steve Guidi Aug 22 '12 at 17:18
ParentObject is defined in the web service, I suppose I could inherit it to do as you suggest, but I don't think that will work well in this context. It looks like I'm back to using public void ProcessParent(). That just means every utility I write starts with a Try{}, followed by 2 foreachs, 2 usings, and "//do something with o" becomes a call to the real utility. Was hoping there was something nicer : ) – ep1033 Aug 22 '12 at 17:20
1

If ParentObject is disposable and it aggregates (and owns) disposable types, then the design convention is that ParentObject.Dispose() disposes the aggregated objects. I would verify that this is the case as it will simplify some of your implementation. Also, if ChildObject has a reference to ParentObject, then you could potentially use IEnumerable.SelectMany() to flatten the loops, and then writing your own Dispose() method to dispose a child's parent. – Steve Guidi Aug 22 '12 at 17:29
Hey, you're right. I reflected into ParentObject, and it keeps track of all open ChildObjects and the dispose method calls to them close them as well. I was never aware of this convention. That certainly does simplify things a bit, though I imagine when looping, I'd probably want to close them more quickly. – ep1033 Aug 22 '12 at 17:35

Humberto · Answer 2 · 2012-08-22T17:20:45.133

3

The "safe" method may not be much safer after all. What if the iteration breaks (or fails) before all parents and children objects are iterated? The remaining objects won't be disposed (at least, not in that specific method).

It seems that the iterations and disposals should be kept separate. You'll have cleaner code and more control over what the program is doing.

And there's more...

The C# iterator pattern will make the "safe" method fail in a subtle way. After you yield the child object, the program will effectively "exit" the using {...} block, thus disposing the child, making it unusable to whoever got it from iterating SafelyGetNextObjInWebApp().

What could be done

Take the using statements out of SafelyGetNextObjInWebApp(). Encapsulate the yielded children objects in a "Unit of Work" class that "knows" when to dispose the child.

edited Aug 22 '12 at 17:20

answered Aug 22 '12 at 17:06

Humberto

7,117
4
31
46

My problem is that against this api, in nearly everything I write, I need to iterate 2 or 3 levels deep against disposable objects. Rather than have my main statement of every utility start 4 tabs deep only to call a Run() method, it would be great if I could find some sort of simpler way of doing this, or banishing it to a separate method. – ep1033 Aug 22 '12 at 17:09
1

The Linq method SelectMany() will collapse several level iterations into one statement. However, you lose any references to the parent objects that would otherwise have been exposed through a foreach statement. This shouldn't be a problem if the child objects have a reference to the parents though. – Steve Guidi Aug 22 '12 at 17:24

score 1 · Answer 3 · answered Aug 22 '12 at 17:03

1

In the end of your block using you execute a dispose, so the same result between using or use dispose in the end of your function

answered Aug 22 '12 at 17:03

Aghilas Yakoub

28,516
5
46
51

So it is safe? as is foreach{ foreach{ yield return; dispose;} dispose;} ? – ep1033 Aug 22 '12 at 17:05
you use using or dispose only for ressource non managed as connection to database or stream .. – Aghilas Yakoub Aug 22 '12 at 17:06
for managed ressource you don't need to execute using, let Garbage Collector – Aghilas Yakoub Aug 22 '12 at 17:07
Yes, in this example we are assuming ChildObject and ParentObject are objects that need to be disposed, as this is what the api tells me. In the code I wrote above, there is an error, is the end block of the using statement still executed? – ep1033 Aug 22 '12 at 17:10
Ok ep1033 i inderstand, place using on webApp and iterate on webApp, and delete other using on childrens – Aghilas Yakoub Aug 22 '12 at 17:12
Webapp does not need to be disposed, just childObject and ParentObject – ep1033 Aug 22 '12 at 17:13
Not according to these other answers – ep1033 Aug 22 '12 at 17:15
1

@Candie: The code is *not* good if only some of the collection elements are iterated. – Steve Guidi Aug 22 '12 at 17:22
The `yield` statement returns from the function immediately, resuming only on the next call. If the caller invokes `SafelyGetNextObjInWebApp()` then only gets the first item from the enumeration (via `.First()`), then the enclosing `using` statement is never exited. – Steve Guidi Aug 22 '12 at 17:34

score 1 · Answer 4 · answered Aug 22 '12 at 17:28

Your code may be ok, but likely cause problems in normal use: you are returning objects that will be disposed on next iteration. So if your caller's code look like

foreach(var i in SafelyGetNextObjInWebApp())
{
   if (IsInteresting(i))
   { 
     interestingItems.Add(i);
   }
}
// here interestingItems contains disposed items you can't use.

Reversing code by providing method that iterates all items and takes Action<T> as argument may highlight the fact that processing of each item must be finished inside action.

iterating through disposable objects

4 Answers4