All IAsyncEnumerable LINQ operators throws OperationCanceledException in GetAsyncEnumerator when token is already canceled.

[neuecc]

Compiler generated enumerator does not throws exception on GetAsyncEnumerator.
But all Async-LINQ operator throws OperationCanceledException when pass canceled token.

static async IAsyncEnumerable<int> FooAsync([EnumeratorCancellation]CancellationToken cancellationToken = default)
{
    yield return 1;
    await Task.Delay(10, cancellationToken);
}

static void Main(string[] args)
{
    // Create Canceled token.
    var cts = new CancellationTokenSource();
    cts.Cancel();

    // OK, don't throw.
    var e1 = FooAsync(cts.Token).GetAsyncEnumerator(cts.Token);
    Console.WriteLine("OK:FooAsyunc().GetAsyncEnumerator()");

    // Ix.Async LINQ Operator throws OperationCanceledException
    var e2 = FooAsync(cts.Token).Select(x => x).GetAsyncEnumerator(cts.Token);
}

Is this the expected behavior?
The source code has a reference to [LDM-2018-11-28].

public IAsyncEnumerator<TSource> GetAsyncEnumerator(CancellationToken cancellationToken)
{
    cancellationToken.ThrowIfCancellationRequested(); // NB: [LDM-2018-11-28] Equivalent to async iterator behavior.

However, I think it doesn't equivalent behavior.

---

Environment:
<TargetFramework>netcoreapp3.1</TargetFramework>
<PackageReference Include="System.Interactive.Async" Version="4.1.1" />

[clairernovotny]

@stephentoub @bartdesmet @MadsTorgersen to comment on the behaviors.

[bartdesmet]

The comment in the code is a reference to the second decision over at https://github.com/dotnet/csharplang/blob/master/meetings/2018/LDM-2018-11-28.md.

In the constructed iterator state machine, GetEnumerator should throw if the given CancellationToken is cancelled. It's not decided exactly where or how often we will check for cancellation.

This is indeed contrary to the implementation of async iterators today. I haven't found an LDM note that reverts this decision, so I'm not sure what the final behavior should be. Either way, we should be aligned with async iterator behavior.

Also adding @jcouv.

[bartdesmet]

Upon digging a bit more, it seems that it was decided to not emit token checks, see dotnet/roslyn#24037:

Generate token checks (answer: no)

and have authors of async iterators use an [EnumerationCancellation] annotated parameter to grab a token, allowing them to emit token checks themselves.

Obviously, we won't change the signature of LINQ methods to have this additional (optional) parameter on IAsyncEnumerable<T> combinators. But the implementations of these operators either directly implement GetAsyncEnumerator(CancellationToken) or can use a local async iterator function that uses the attribute:

public static IAsyncEnumerable<R> Select<T, R>(this IAsyncEnumerable<T> source, Func<T, R> selector)
{
    return Core();

    async IAsyncEnumerable<R> Core([EnumeratorCancellation]CancellationToken token = default)
    {
        await foreach (var item in source.WithCancellation(token).ConfigureAwait(false))
        {
            yield return selector(item);
        }
    }
}

We didn't get around to use the latter pattern yet, because custom attributes on local functions were not supported at the time (they're now in preview).

That leaves the question on when to perform token checks. One option that's cheap and relies on the compositional nature of query operators could be to delegate the token to query operator sources whenever possible (we already do), and don't have checks in the LINQ operators directly (thus cheap). Sources will need to be trusted to cooperate in honoring cancellation, so the query operators won't improve on a source's behavior.

If we go that way, generators on AsyncEnumerable (i.e. ones that act as sources and aren't combinators on existing sequences) would need to have token checks, so they can be trusted to honor cancellation.

In addition, there could be an opt-in helper "operator" that injects checks for cancellation if one is dealing with a source that doesn't cooperate, e.g.:

public static IAsyncEnumerable<T> ThrowIfCancellationRequested<T>(this IAsyncEnumerable<T> source)
{
    return Core();

    async IAsyncEnumerable<T> Core([EnumeratorCancellation]CancellationToken token = default)
    {
        token.ThrowIfCancellationRequested();

        await foreach (var item in source.WithCancellation(token).ConfigureAwait(false))
        {
            yield return item;

            token.ThrowIfCancellationRequested();
        }
    }
}

This way, we keep the operators cheap (checking for cancellation incurs a volatile read), and have an opt-in mechanism for frequent cancellation checking, but generally defer to sources to honor cancellation at their earliest convenience.

[clairernovotny]

@bartdesmet We're clear to use the latest compiler (with support for attributes on local functions) for building this library. Who has the next action here?

[bartdesmet]

I'll look into leveraging the new language/compiler functionality where appropriate.

We can continue to discuss the desired behavior with regards to cancellation checking in query operators. I'm leaning towards what I wrote earlier, but happy to hear other points of view.