LINQ, Union followed by Select on empty IEnumerable fails to invoke Dispose

[acolombi]

It is my understanding that LINQ should always call IEnumerator's Dispose method when finished with an IEnumerator. I've found a case where this does not happen: Take two IEnumerators that immediately return false in their MoveNext() implementations (i.e. they are empty), Union them and then perform a Select on that. Here is a minimal reproduction that shows Dispose() is not called for either IEnumerator. This reproduces in dotnet core 2.1 & 2.2. It does not reproduce in .NET Framework 4.7.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.

[gabeluci]

This was first asked on Stack Overflow, so I did some digging.

1. This has nothing to with Select(), just Union() (but not if you use Union().ToList() since that uses a different implementation).
2. This only occurs for empty collections. More specifically, when MoveNext() never returns true.
3. I believe the bug is on line 105 of Union.cs. SetEnumerator() is only ever called after enumerator.MoveNext() returns true. So for an empty collection, it would never get set. Then in Dispose(), there is no enumerator saved that it can call Dispose() on.

When you call Union().ToList(), it ends up using UnionWith(), which relies on foreach, so it's not a problem there.

The .NET Framework implementation also relies on foreach, so it's not a problem there either.

Here are reproductions of it:

• .NET Core 2.2
• .NET Framework 4.7.2

[scalablecory]

Thanks for the report. Would you care to submit a PR?

[gabeluci]

Sorry about the radio silence on this. I thought I might have time to give it a try, but it just didn't happen. I'm glad someone picked it up.

[stephentoub]

Fixed in dotnet/corefx#40384

[GSPP]

Is there enough test coverage to ensure that disposing happens reliably? Some sequences hold up resources such as database connections or file handles. If those are dropped this can be a nasty surprise (especially suddenly under load in production).

I understand that the LINQ implementations have become a lot more complex internally due to various optimizations. There's now more potential for forgetting to dispose. Most queries are in-memory so this would not be noticed.

Update: I now see the referenced pull request that appears to add comprehensive coverage. 🍾