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) #32684

Closed
stephentoub opened this Issue Oct 8, 2018 · 5 comments

Comments

Projects
None yet
3 participants
@stephentoub
Copy link
Member

stephentoub commented Oct 8, 2018

Related to #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

This comment has been minimized.

Copy link
Collaborator

jnm2 commented Oct 8, 2018

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

@stephentoub

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Member

stephentoub 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?

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

This comment has been minimized.

Copy link
Member

stephentoub commented Nov 8, 2018

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

Good point. I've opened #33336.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment