2

I am experiencing a strange issue while enumerating an IAsyncEnumerable, with the System.Linq.Async operator Take attached to it. In my iterator I have a try-finally block with some values yielded inside the try block, and some clean-up code inside the finally block. The clean-up code is inside a lock block. The issue is that whatever code follows the lock block is not executed. No exception is thrown, the code is just ignored like it's not there. Here is a program that reproduces this behavior:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;

public class Program
{
    static async Task Main()
    {
        await foreach (var item in GetStream().Take(1))
        {
            Console.WriteLine($"Received: {item}");
        }
        Console.WriteLine($"Done");
    }

    static async IAsyncEnumerable<int> GetStream()
    {
        var locker = new object();
        await Task.Delay(100);
        try
        {
            yield return 1;
            yield return 2;
        }
        finally
        {
            Console.WriteLine($"Finally before lock");
            lock (locker) { /* Clean up */ }
            Console.WriteLine($"Finally after lock");
        }
    }
}

Output:

Received: 1
Finally before lock
Done

The text "Finally after lock" is not printed in the console!

This only happens with the Take operator attached. Without the operator the text is printed as expected.

Is this a bug in the System.Linq.Async library, a bug in the C# compiler, or something else?

As a workaround I am currently using a nested try-finally block inside the finally, which works but it's awkward:

finally
{
    try
    {
        lock (locker) { /* Clean up */ }
    }
    finally
    {
        Console.WriteLine($"Finally after lock");
    }
}

.NET Core 3.1.3, .NET Framework 4.8.4150.0, C# 8, System.Linq.Async 4.1.1, Visual Studio 16.5.4, Console Application

Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
  • 1
    Very interesting behavior. The code within the lock is executed; the same thing happens for `Take(2)`; for `Take(3)`, the `"Finally after lock"` is emitted. – janw May 03 '20 at 16:05
  • You should report it at https://github.com/dotnet/reactive – Paulo Morgado May 03 '20 at 18:08
  • Small question. Why do you need lock there? – Guru Stron May 03 '20 at 19:27
  • @GuruStron because I add an object in a collection at the start of the enumeration, and remove it when the enumeration is finished. And using a `lock` is the simplest way to ensure thread safety. – Theodor Zoulias May 03 '20 at 19:56
  • @TheodorZoulias I was a little bit confused cause lock object was local to the method in the repro =). Also it is up for debate but personally I would say using [concurrent collection](https://learn.microsoft.com/en-us/dotnet/api/system.collections.concurrent?view=netcore-3.1) would be the simplest one) – Guru Stron May 03 '20 at 19:59
  • @GuruStron yeap, the code I posted is just for demonstrating the issue, and doesn't make much sense otherwise. Regarding using a concurrent collection, I have considered it but this particular collection must be accessed with snapshot semantics. So I ended up using an [`ImmutableArray`](https://learn.microsoft.com/en-us/dotnet/api/system.collections.immutable.immutablearray). Here is the actual code in the finally block: `lock (_locker) _subscribers = _subscribers.Remove(subscriber);` – Theodor Zoulias May 03 '20 at 20:08

1 Answers1

3

Would not claim that I fully understand the issue and how to fix it(and whose fault is it) but that's what I found:

First of all the finally block is translated to next IL:

  IL_017c: ldarg.0      // this
  IL_017d: ldfld        bool TestAsyncEnum.Program/'<GetStream>d__1'::'<>w__disposeMode'
  IL_0182: brfalse.s    IL_0186
  IL_0184: br.s         IL_0199
  IL_0186: ldarg.0      // this
  IL_0187: ldnull
  IL_0188: stfld        object TestAsyncEnum.Program/'<GetStream>d__1'::'<>s__2'

  // [37 17 - 37 58]
  IL_018d: ldstr        "Finally after lock"
  IL_0192: call         void [System.Console]System.Console::WriteLine(string)
  IL_0197: nop

  // [38 13 - 38 14]
  IL_0198: nop

  IL_0199: endfinally
} // end of finally

As you can see the compiler generated code has next branching IL_017d: ldfld bool TestAsyncEnum.Program/'<GetStream>d__1'::'<>w__disposeMode' which will run the code after lock statement only if the generated enumerator is not in the disposeMode.

System.Linq.Async has two operators which internally use AsyncEnumerablePartition - Skip and Take. Difference is that when Take finishes it does not run underlying enumerator to completion, and Skip does (I have elaborated here a little bit, cause have not looked in the underlying implementation), so when the disposal code is triggered for Take case the disposeMode is set to true and that part of code is not run.

Here is class (based on what's happening in the nuget) to repro issue:

public class MyAsyncIterator<T> : IAsyncEnumerable<T>, IAsyncEnumerator<T>
{
    private readonly IAsyncEnumerable<T> _source;
    private IAsyncEnumerator<T>? _enumerator;
     T _current = default!;
    public T Current => _current;

    public MyAsyncIterator(IAsyncEnumerable<T> source)
    {
        _source = source;
    }

    public IAsyncEnumerator<T> GetAsyncEnumerator(CancellationToken cancellationToken = new CancellationToken()) => this;

    public async ValueTask DisposeAsync()
    {
        if (_enumerator != null)
        {
            await _enumerator.DisposeAsync().ConfigureAwait(false);
            _enumerator = null;
        }
    }

    private int _taken;
    public async ValueTask<bool> MoveNextAsync()
    {
        _enumerator ??= _source.GetAsyncEnumerator();

        if (_taken < 1 && await _enumerator!.MoveNextAsync().ConfigureAwait(false))
        {
            _taken++; // COMMENTING IT OUT MAKES IT WORK
            _current = _enumerator.Current;
            return true;
        }

        return false;
    }
}

And usage in your code await foreach (var item in new MyAsyncIterator<int>(GetStream()))

I would say that this is some edge case compiler issue, cause it seems to handle strangely all code after finally blocks, for example if your add Console.WriteLine("After global finally"); to the end of GetStream it will not be printed also in case if the iterator has not "completed". Your workaround works cause the WriteLine is in finally block.

Submitted issue on github, will see what dotnet team will say.

Guru Stron
  • 102,774
  • 10
  • 95
  • 132
  • Thanks Stron for the answer. Very interesting analysis and observations! Also thanks for [submiting](https://github.com/dotnet/runtime/issues/35779) the issue to GitHub (I am too lazy to do it myself!). – Theodor Zoulias May 03 '20 at 22:16
  • 1
    NP, it was fun and educational for me =) – Guru Stron May 03 '20 at 22:19