Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should / how should CancellationToken show up in IAsyncEnumerable-related interfaces? #27858

Closed
stephentoub opened this issue Nov 8, 2018 · 3 comments
Labels
api-approved API was approved in API review, it can be implemented area-System.Runtime
Milestone

Comments

@stephentoub
Copy link
Member

Since the rest of the support is set, and since we've now merged these interfaces in support of .NET Core 3.0 previews, I'm opening a separate issue specifically to cover whether we should make any changes related to cancellation.

See https://github.com/dotnet/corefx/issues/32640#issuecomment-436316907.

@stephentoub
Copy link
Member Author

stephentoub commented Dec 6, 2018

We discussed this again at length in C# language design, and came to the conclusion that we should add back the CancellationToken argument to GetAsyncEnumerator:
https://github.com/dotnet/csharplang/blob/master/meetings/2018/LDM-2018-11-28.md#async-iterators-and-await-foreach

That means we change this:

public interface IAsyncEnumerable<out T>
{
    IAsyncEnumerator<T> GetAsyncEnumerator();
}

to this:

public interface IAsyncEnumerable<out T>
{
    IAsyncEnumerator<T> GetAsyncEnumerator(CancellationToken cancellationToken = default);
}

The language/compiler will do a minimal amount to support this:

  • await foreach will just use GetAsyncEnumerator(), relying on the default non-cancelable token. If a developer wants to pass a token in while enumerating, they can either skip the await foreach and just use GetAsyncEnumerator/MoveNextAsync/Current directly, or they can use a WithCancellation-ish extension method that we'll want to add, e.g. await foreach (var item in source.WithCancellation(token)) { … }. That extension will just return a struct that exposes the async enumerable pattern, but with the token baked into it.
  • For async iterators, the compiler will generate GetAsyncEnumerator to do cancellationToken.ThrowIfCancellationRequested(). Still needs discussion, but it may also generate such a call as part of each MoveNextAsync on the iterator.

This isn't a perfect solution, but it's the best of the options we're aware of, it enables composability and combinators and the like, and it leaves the door open for the compiler to do more in the future if it proves important. This leads to the following approach/guidance for developers:

  • If you're just writing internal async iterators for use in your own codebase, you can of course continue to pass CancellationTokens to your iterator methods and just use them as you would any other token, e.g.
internal static async IAsyncEnumerable<Stuff> EnumerateStuff(CancellationToken cancellationToken)
{
    await foreach (OtherStuff item in EnumerateOtherStuff(CancellationToken))
    {
        yield return GetStuff(item);
    }    
}
  • However, that doesn't play nicely with anyone passing a token via the interface, so if you're exposing public APIs, the IAsyncEnumerable-returning method shouldn't take in the caller's CancellationToken, as it should instead be taken in via GetAsyncEnumerator. The easiest way to write that is by writing the GetAsyncEnumerator method as an async iterator:
public static async IAsyncEnumerable<Stuff> EnumerateStuff() =>
    new EnumerateStuffIterator();

internal sealed class EnumerateStuffIterator : IAsyncEnumerable<Stuff>
{
    public async IAsyncEnumerator<Stuff> GetAsyncEnumerator(CancellationToken cancellationToken)
    {
        await foreach (OtherStuff item in EnumerateOtherStuff(cancellationToken))
        {
            yield return GetStuff(item);
        }
    }
}

While a small amount of additional boilerplate, this will then compose nicely with an ecosystem that provides the token to the interface call. One additional downside to this "easy" approach is it results in one more allocation that we could otherwise get away with, but our hope is that for an async iteration that one additional allocation should be nominal, and for critical things where it isn't, it's still possible to avoid it by writing the state machine manually.

I will open a separate issue for WithCancellation: we'll need to think through the interplay of that with ConfigureAwait.

cc: @onovotny, @jcouv, @terrajobst, @bartonjs

@andriysavin
Copy link

andriysavin commented Dec 6, 2018

@stephentoub Thank you for posting details. Could you make the last code sample more explicit regarding how the CancellationToken gets to the EnumerateStuffIterator.GetAsyncEnumerator()?

@stephentoub
Copy link
Member Author

It should have been a parameter. Fixed.

@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 3.0 milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Runtime
Projects
None yet
Development

No branches or pull requests

3 participants