My understanding is that LINQ IEnumerable extensions are supposed to call Dispose on the IEnumerators produced by the IEnumerable. See this answer on SO. However, I'm not seeing that in practice. Is the other SO answer incorrect, or have I found a bug in LINQ?
Here's a minimal reproduction of the issue using Mono. It also reproduces in Dotnet Core 2.1 & 2.2.
using System;
using System.Threading;
using System.Linq;
using System.Collections.Generic;
using System.Collections;
namespace union
{
class MyEnumerable : IEnumerable<long>
{
public IEnumerator<long> GetEnumerator()
{
return new MyEnumerator();
}
IEnumerator IEnumerable.GetEnumerator()
{
return GetEnumerator();
}
}
class MyEnumerator : IEnumerator<long>
{
public long Current
{
get
{
return 0;
}
}
object IEnumerator.Current
{
get
{
return Current;
}
}
public bool MoveNext()
{
return false;
}
public void Reset()
{
return;
}
public void Dispose()
{
Console.WriteLine("I got disposed");
}
}
class Program
{
static void Main(string[] args)
{
var enum1 = new MyEnumerable();
var enum2 = new MyEnumerable();
enum1.Union(enum2).Select(x => x + 1).ToList();
Console.WriteLine("All done!");
}
}
}
If the Dispose was getting called you would see I got disposed
twice on the console. Instead you get no I got disposed
s. The Union
and the Select
are required to reproduce the issue.
Any C# gurus know if I've hit a bug?
Update:
I believe this is a bug, and I've filed: https://github.com/dotnet/corefx/issues/40384.
I tested in .NET Fiddle, which allows you to use one of three compilers, and the results confirm what you found:
Seems like Mono doesn't either.
But I think I narrowed down what's going on: It does not call Dispose()
when the first call to MoveNext()
returns false
(i.e. there are no items in the collection).
You will see this, for example, if you replace your MoveNext()
code with this, which just returns true
the first time and false
thereafter (i.e. simulates a collection with 1 item):
private bool happenedOnce = false;
public bool MoveNext()
{
if (happenedOnce) return false;
happenedOnce = true;
return true;
}
Here is that example in Mono and .NET Core 2.2.
I guess it could be argued that it's not a bug, it's a feature. What's there to dispose if there's nothing in the collection? (Scratch that - files and such...)
But then again, it is inconsistent. For example, this will give you exactly the same results, but it will dispose, even if there is nothing in the collection:
enum1.Select(x => x + 1).Union(enum2.Select(x => x + 1)).ToList();
But all that does is force it to iterate the collection using Select()
instead of Union()
. So that does sound like it's a bug in the implementation of Union()
.
I had the time today (family is away visiting friends) so I took the time to debug the .NET Core source. I believe the issue is right here:
if (enumerator.MoveNext())
{
SetEnumerator(enumerator);
StoreFirst();
return true;
}
SetEnumerator()
stores the enumerator in an instance variable. You see it only gets called if MoveNext()
returns true
. Later, in Dispose()
, it only calls Dispose()
on the iterator if it's not null
. But since it never got set, it is indeed null
.
Contrast that with the implementation of Select()
, which sets its _enumerator
variable before calling MoveNext()
.
The .NET Framework just relies on foreach
, as you can see here. Oddly enough, so does Mono, so I don't know what's up there.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With