2

These two questions almost answer my own question, but not quite. Consider this a follow-up question to these.

I understand that a foreach loop will Dispose of an enumerator when it's done with it.

My question is this:

If the enumerator in question is actually a c# iterator block (i.e. GetEnumerator()/yield) of an IEnumerable<T> class which itself implements IDisposable, can I be sure that the object itself will be disposed? Or do I need to resort to calling GetEnumerator()/MoveNext()/Current explicitly inside a using block?

EDIT: This snippet demonstrates @JonSkeet's answer.

EDIT #2: This snippet, based on @JonSkeet's comments, demonstrates ideal usage. The iterator block is responsible for the lifetime of needed resources, not the enumerable object itself. This way, the enumerable can be enumerated multiple times if need be -- each enumerable has its own resource to work with.

Community
  • 1
  • 1
CSJ
  • 3,641
  • 2
  • 19
  • 29
  • Can you post a SSCCE? (Doing so and running it might answer the question anyway.) I don't quite follow the question: no actions with an enumerator will cause the collection object it belongs to to be disposed. – TypeIA Mar 05 '14 at 15:04
  • The caller cannot tell how the iterator method was implemented. A different .NET language might not even know that concept. – usr Mar 05 '14 at 15:04
  • 3
    Are you asking if doing `foreach` over an `IEnumerator where T : IEnumerable, IDisposable` will dispose the objects of type `U`? Of course not, why would it do that? If I enumerate a collection of open sockets should the sockets be automatically closed? – Jon Mar 05 '14 at 15:04
  • It's not really clear what you mean - are you trying to dispose of each object returned via the iterator? – Jon Skeet Mar 05 '14 at 15:04
  • @JonSkeet No, I'm trying to ensure that the Enumerable is disposed. Imagine that its constructor gets a file handle and the iterator block yields each line in the file. – CSJ Mar 05 '14 at 15:06
  • @CSJ You already know that. I know that you know that because you say so right in your question, "I understand that a foreach loop will Dispose of an enumerator when it's done with it." – Servy Mar 05 '14 at 15:07
  • @Servy No, I know that the anonymous thing doing the enumerating will be disposed; it's not clear whether the object that served up the enumerator (i.e. the enumerable) will also be disposed. That's my question. – CSJ Mar 05 '14 at 15:08
  • 1
    @CSJ It doesn't implement `IDisposable`, and *has* no `Dispose` method, so it *cannot* be disposed. `IEnumerator` extends `IDisposable`, not `IEnumerable`. – Servy Mar 05 '14 at 15:10
  • @Servy In my original question I stipulate that my IEnumerable also implements IDisposable explicitly. – CSJ Mar 05 '14 at 15:11
  • @CSJ It's an iterator block. You couldn't make it do that even if you tried. It's a compiler generated class. If you had a custom object, i.e. **not** an iterator block, then it could implement `IDisposable`, and `foreach` would not dispose of it. – Servy Mar 05 '14 at 15:13

2 Answers2

6

foreach calls Dispose on the iterator regardless of the implementation of that iterator. It doesn't matter if it was created with an iterator block or not, it's Dispose method is called either way.

That's the whole point of interfaces, such as IDisposable. You don't need to care what the underlying implementation is. It always disposes of everything. What they choose to do with that call is up to them.

As for the IEnumerable<T> (not the IEnumerator<T>) generated by the iterator block, it will never implement IDisposable, and thus cannot be disposed. If you have a custom object (rather than an iterator block) that implements IEnumerable<T> and IDisposable, then it will not be disposed of when used in a foreach. Only the IEnumerator<T> created through GetEnumerator will be.

Servy
  • 202,030
  • 26
  • 332
  • 449
3

If you want to dispose of the IEnumerable<T>, you need to do that yourself with a using statement (or manual try/finally). A foreach loop does not dispose of the IEnumerable<T> automatically - only the iterator it returns.

So, you'd need:

using (var enumerable = new MyEnumerable())
{
    foreach (var element in enumerable)
    {
        ...
    }
}

It makes no difference whatsoever whether MyEnumerable implements IEnumerable<T> using an iterator block or some other code.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • I was afraid of that. An alternative is to do any cleanup inside the iterator block itself before it quits (e.g. by calling `this.Dispose()` directly in front of `yield break`, for example) – CSJ Mar 05 '14 at 15:18
  • 1
    @CSJ: You would have to do something like `using (this) { ... }` - don't forget that `foreach` may not iterate over the whole of the iterator's contents. I would strongly suggest that you *don't* do this, however. It would be very non-idiomatic, to say the least, for iterating over a sequence to dispose of the sequence itself. It sounds like a reasonably odd design to start with... – Jon Skeet Mar 05 '14 at 15:20
  • I see. It seems I'm more interested in building an enumerator than an enumerable. – CSJ Mar 05 '14 at 15:24
  • @CSJ Again, it's not *possible* to write an iterator block that generates an `IEnumerable` that is disposable, so having it dispose of itself inside of the iterator block is a moot point. Not that that's the only roadblock. It would need to be a non-iterator block `IEnumerable`. And on top of that, there would need to be some way for the `IEnumerator` to know what `IEnumerable` created it. It's a potentially solvable problem, but still bad practice, and entirely unnecessary. If you're in such a situation simply don't create the disposable resource in the `IEnumerable`. – Servy Mar 05 '14 at 15:25
  • @Servy: Actually, in practice the `IEnumerable` returned from an iterator block *does* implement `IDisposable`. It's a bit of a hack for the sake of efficiency. But I agree that one shouldn't *expect* it to :) – Jon Skeet Mar 05 '14 at 15:31
  • @JonSkeet Well, in any case, you couldn't run any of your code when it's disposed, so it may as well not be `IDisposable` from his perspective. – Servy Mar 05 '14 at 15:33
  • I'm trying to create an experience similar to `foreach (var line in File.ReadAllLines()) { ... } without forcing the caller to dispose anything, but not leaving any handles lying around. Looks like the object returned should be an enumerator, not an enumerable. – CSJ Mar 05 '14 at 15:38
  • @CSJ: If you *only* open the handles within the iterator block, and do that within a `using` statement, it'll be fine. When the iterator is disposed, that will invoke any appropriate `finally` blocks (including implicit ones from `using` statements) in the iterator block. – Jon Skeet Mar 05 '14 at 15:42
  • @CSJ The key there is that the `IEnumerable` doesn't open up any resources. It is the `IEnumerator` that actually opens up the file. `File.ReadLines` returns an `IEnumerable` (not an `IEnumerator`) that, each time it is asked to create a new `IEnumerator`, opens a new file. Follow that pattern. You don't need to (and likely don't want to) have your method itself return an `IEnumerator`. – Servy Mar 05 '14 at 16:08
  • @Servy: I completely agree with the sentiment - but `File.ReadLines` is a bad example because it's implemented so badly. If it had been implemented appropriately, everything you say would be true :) – Jon Skeet Mar 05 '14 at 16:11
  • @JonSkeet We're getting a bit offtopic here, but how does it differ from that? – Servy Mar 05 '14 at 16:14
  • @Servy: By the looks of it, it actually opens the file *immediately* - and maintains a single reader however many times you call `GetEnumerator` on it. Try `var lines = File.ReadLines("test.txt"); foreach (var x in lines) { foreach (var y in lines) { Console.WriteLine(x + " " + y); } }` and see what happens with a `test.txt` of `n` lines. It *should* show `n^2` lines... but it doesn't :( – Jon Skeet Mar 05 '14 at 16:17
  • @JonSkeet Ah, yeah. Apparently it handles subsequent reads just fine: `lines.Count();lines.Count();` is fine. The issue is that calling `GetEnumerator` *while another iterator is currently iterating the file* returns the same handle, rather than a separate one. Calling `GetEnumerator` after the previous call finishes isn't a problem. A slightly simpler way of demonstrating this than your code is `foreach (var line in lines) Console.WriteLine(lines.Count());`, which throws an object disposed exception, instead of just printing the number of lines N times. – Servy Mar 05 '14 at 16:22
  • @Servy: Right. And the fact that `var lines = File.ReadLines("test.txt");` opens the file *at all* is a problem, too. Bah. – Jon Skeet Mar 05 '14 at 16:24
  • @JonSkeet I don't *think* that does though. At the very least, I can't prove that it does. I *believe* that it doesn't open the file until you first start iterating it. It's just that it stores the file in some state shared between all `IEnumerator` instances. Most likely this is because it returns an object that implements both `IEnumerable` and `IEnumerator`, and the `Enumerable` returns itself. So (if I'm right) it's not *so* bad (it's still not ideal though). – Servy Mar 05 '14 at 16:26
  • @Servy: I'll see if I can prove it to you... watch this space :) – Jon Skeet Mar 05 '14 at 16:32
  • @JonSkeet nevermind, you're right. It does open the file. Simple example: `var handle = File.OpenWrite("output.txt"); var lines = File.ReadLines("output.txt");` (that throws an exception, it ideally shouldn't until `lines` creates an iterator. – Servy Mar 05 '14 at 16:35
  • @Servy: Right. I was going to try it the other way round: call ReadLines then Delete :) – Jon Skeet Mar 05 '14 at 16:40
  • 1
    @Servy, JonSkeet: Very interesting discussion, clearly pointing out the respective responsibilities of the enumerator vs. the enumerable. So a reference example to _avoid_. – CSJ Mar 05 '14 at 16:41
  • BTW, calling `Dispose` on an iterator before `GetEnumerator` has called has no effect; calling it after that time will dispose the very first enumerator created if it hasn't been disposed yet. – supercat Mar 06 '14 at 16:11
  • @supercat: By "an iterator" do you mean "the return returned from a method implemented with an iterator block"? If so, then I agree. – Jon Skeet Mar 06 '14 at 16:14
  • @JonSkeet: That is indeed what I meant. The behavior is mostly harmless, but it illustrates one of the difficulties with assuming that object *instances* which happen to implement `IDisposable` need cleanup. – supercat Mar 06 '14 at 16:31