Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

LINQ isn't calling Dispose on my IEnumerator when using Union and Select, expected behavior or bug?

Tags:

c#

linq

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 disposeds. 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.

like image 572
Andrew Avatar asked Aug 17 '19 00:08

Andrew


1 Answers

I tested in .NET Fiddle, which allows you to use one of three compilers, and the results confirm what you found:

  • .NET 4.7.2 does dispose
  • Roslyn 2.0 does dispose
  • .NET Core 2.2 does not

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.

like image 156
Gabriel Luci Avatar answered Oct 10 '22 22:10

Gabriel Luci