Fix problem with finally in UniTaskAsyncEnumerable.Create not being executed

[hadashiA]

#438

If await foreach break in the middle, the factory declared in UniTaskAsyncEnumerable.Create(...) is not executed until the end.

await foreach (var elem in IterateAsync())
{
    Console.WriteLine("elem: " + elem);
    break;
}

static IUniTaskAsyncEnumerable<int> IterateAsync()
{
    return UniTaskAsyncEnumerable.Create<int>(async (writer, _) =>
    {
        try
        {
            await writer.YieldAsync(0);
        }
        finally
        {
            Console.WriteLine("Finally block"); // Never executes.
        }
    });
}

Perhaps the cause is that YieldAsync() is not completing; Calling YieldAsync() immediately breaks the await foreach, so the AsyncEnumerable is disposed; the YieldAsync awaiter state machine never calls the last MoveNext.

I added a AsyncWriter.Dispose so that it will complete if YieldAsync is in a pending state.

[timcassell]

I don't think this actually fixes the issue.

static IUniTaskAsyncEnumerable<int> IterateAsync()
{
    return UniTaskAsyncEnumerable.Create<int>(async (writer, _) =>
    {
        try
        {
            await writer.YieldAsync(0);
            // Nothing should execute here after `break`, it should skip right to the finally block.
        }
        finally
        {
            // If you `await` something else inside the finally block, the `DisposeAsync` should wait for it.
            Console.WriteLine("Finally block"); // Never executes.
        }
    });
}

But, this may be impossible to actually get the same behavior as C#8 async iterators without compiler support...

[hadashiA]

Oh, you are right ...
Thanks for the suggestion.

// If you await something else inside the finally block, the DisposeAsync should wait for it.

Maybe this cannot be supported.

// Nothing should execute here after break, it should skip right to the finally block.

By the way, I think this is supported by this PR. right..?

[timcassell]

By the way, I think this is supported by this PR. right..?

Yes and no...

try
{
    await writer.YieldAsync(0);
    Console.WriteLine("After yield"); // Correctly does not execute due to the exception
}
// This should technically not hit the catch block and only go to the finally block,
// but that's not really possible without compiler support.
// [Edit] Actually this should not be allowed at all, see comment below.
catch (Exception e)
{
    Console.WriteLine($"exception: {e}"); // Prints System.OperationCanceledException
}
finally
{
    await Task.Delay(100); // Does not wait
    Console.WriteLine("Finally block");
}

// If you await something else inside the finally block, the DisposeAsync should wait for it.

Maybe this cannot be supported.

This I actually think can be supported, the DisposeAsync should just wait for the original task to complete (but swallow the cancellation exception).

[Edit] Also, C# disallows using yield keyword inside a try block with a catch block, so that's technically a bad usage of yield anyway (except we don't get the handy compiler support to block it). https://learn.microsoft.com/en-us/dotnet/csharp/misc/cs1626
Possibly an analyzer could prevent such usage.

[timcassell]

@hadashiA I implemented the support for it in my library: timcassell/ProtoPromise#276. I also added an analyzer for that case I mentioned: timcassell/ProtoPromise#296.

I know UniTask has a different structure than mine, but you could take a look for inspiration if you want. 😉