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

Proposal: IAsyncEnumerable.ConfigureAwait(bool continueOnCapturedContext) #27565

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

Comments

@stephentoub
Copy link
Member

Related to https://github.com/dotnet/corefx/issues/32640.

Background

With the addition of IAsyncEnumerable<T>, developers will be able to write asynchronous foreach loops, e.g.

foreach await (T item in source)
{
    ProcessItem(item);
}

This loop expands by the compiler into the equivalent of:

IAsyncEnumerator<T> e = source.GetAsyncEnumerator();
try
{
    while (await e.MoveNextAsync())
    {
        T item = e.Current;
        ProcessItem(item);
    }
}
finally
{
    await e.DisposeAsync();
}

Note that as part of that expansion, the compiler introduced several awaits not visible in the original source code (which is part of why a developer annotates the foreach as foreach await). This introduces an issue, in that a developer now no longer has explicit syntactic control for that await operation, which means that there’s nowhere for a developer to append ConfigureAwait(false) if they want to prevent the await from grabbing the current context and scheduling the continuation back to it. Our guidelines are that library code by default should use ConfigureAwait(false) on every await, so the lack of that here is problematic for library authors.

Proposal

Allow a developer to write:

foreach await (T item in source.ConfigureAwait(false))
{
    ProcessItem(item);
}

This would be achieved with an extension method as follows:

namespace System.Collections.Generic
{
    public static class AsyncEnumerable
    {
        public static ConfiguredAsyncEnumerable<T> ConfigureAwait(this IAsyncEnumerable<T> source, bool continueOnCapturedContext);
    }
}

namespace System.Runtime.CompilerServices
{
    public struct ConfiguredAsyncEnumerable<T>
    {
        public Enumerator GetAsyncEnumerator();

        public struct Enumerator
        {
            public ConfiguredValueTaskAwaitable<T> MoveNextAsync();
            public T Current { get; }
            public ConfiguredValueTaskAwaitable DisposeAsync();
        }
    }
}

Because foreach await is able to bind by pattern, this ConfiguredAsyncEnumerable<T> can be foreach await’d, and the previously shown expansion will now how each of the awaits operating on a ConfiguredValueTaskAwaitable<T> or ConfiguredValueTaskAwaitable instead of a ValueTask<T> or ValueTask.

Open issues:

  • Do we want this? foreach await is just syntactic sugar for something a developer could otherwise write. However, it’s really useful syntactic sugar, and forcing a developer that wants to iterate through an async enumerable in a library to expand it out manually seems wrong.
  • Extension method type name and namespace. Presumably AsyncEnumerable is a nice name for a LINQ implementation to use. Do we want to claim it for this? But presumably it should also live in the same namespace as IAsyncEnumerable<T>, so that it’s always available when using that interface.
@jnm2
Copy link
Contributor

jnm2 commented Oct 8, 2018

Shouldn't there be an IAsyncDisposable.ConfigureAwait, too?

@stephentoub
Copy link
Member Author

stephentoub commented Oct 8, 2018

Shouldn't there be an IAsyncDisposable.ConfigureAwait, too?

@MadsTorgersen, can you remind me of the plan around using await... presumably it's going to support pattern-based as well?

@stephentoub stephentoub self-assigned this Oct 10, 2018
@KPixel
Copy link

KPixel commented Oct 25, 2018

As an alternative to the static ConfigureAwait() method (which I don't like as a concept), could you handle this the same way you are handling passing the cancellation token, by pushing it outside the scope of IAsyncEnumerable?

That way, each implementation of IAsyncEnumerable can provide its ConfigureAwait() method.

Also, in the future, if you find a better solution, it will be easier to introduce then...

@stephentoub
Copy link
Member Author

As an alternative to the static ConfigureAwait() method (which I don't like as a concept), could you handle this the same way you are handling passing the cancellation token, by pushing it outside the scope of IAsyncEnumerable?

I'm not entirely sure what you mean, as the ConfigureAwait is entirely outside the scope of the IAsyncEnumerable, more so than with CancellationToken. ValueTask, which is returned by MoveNextAsync and DisposeAsync, supports ConfigureAwait already, so this extension method on IAsyncEnumerable is just a way to get the compiler's pattern-matching for await foreach to pass that ConfigureAwait call on to each ValueTask it awaits. It's implemented entirely on top of everything else, without any hooks or internals access or any such things needed.

@stephentoub
Copy link
Member Author

Shouldn't there be an IAsyncDisposable.ConfigureAwait, too?

Good point. I've opened https://github.com/dotnet/corefx/issues/33336.

@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

4 participants