IAsyncEnumerable should not implement IEnumerable

[TomPeters]

I have a suggestion about the API based on an issue I have encountered using this library.

In reference to the following two interfaces:
public interface IAsyncEnumerable : IEnumerable
public interface IAsyncEnumerable<T> : IEnumerable<T>, IAsyncEnumerable

Currently they implement IEnumerable and IEnumerable<T>. I don't think they should.

With the current implementation, it is easy to pass an IAsyncEnumerable around as an IEnumerable. If you are in an asp.net request and are therefore using the asp.net synchronization context, you could create a deadlock if you use an AsyncEnumerable as an IEnumerable.

There are two obvious solutions to this problem:

1. Ensure that you use ConfigureAwait(false) everywhere (and hope that you don't need HttpContext).
2. Do not use the AsyncEnumerable as an IEnumerable (in order to go 'async all the way').

Both of these approaches are error prone, as there is no guarantee you have done either of them correctly, nor is there a good way of tracking down all of the appropriate places that need to be checked.

I suggest changing the IAsyncEnumerable interfaces such that they do not implement IEnumerable to avoid such issues.

I can see that it is very useful to be able to use an IAsyncEnumerable as an Enumerable in many cases, so I can see why it was done this way. But perhaps it would be better for this to be an extension method to transform an IAsyncEnumerable into an IEnumerable?

eg:
public static IEnumerable<T> ToEnumerable(this IAsyncEnumerable<T> asyncEnumerable)

This has two advantages:

1. It makes it explicit that you want to enumerate synchronously
2. It makes it easy to track down places that you are enumerating synchronously.

Point 2 here is particularly useful for the deadlock scenario: If you are find a usage of this extension method within an asp.net request, then you know there is a deadlock risk and you may want to instead enumerate asynchronously, and propagate the async all the way to your entry point.

[kind-serge]

I understand the struggle with putting obnoxious ConfigureAwait(false), but this is the only right solution to make ASP.NET or WPF happy. The AsyncEnumerable internally always does Task.ConfigureAwait(false).GetAwaiter().Result on synchronous enumeration to avoid dead-locks on SynchronizationContext, but this will work properly only if async enumeration function (that is passed into the constructor) does ConfigureAwait(false) on downstream async calls. I.e:

// this might hang on synchronous enumeration in ASP.NET/WPF
new AsyncEnumerable(async yield =>
{
    await FooAsync();
    await yield.ReturnAsync(123);
})

// this will not hang on synchronous enumeration in ASP.NET/WPF
new AsyncEnumerable(async yield =>
{
    await FooAsync().ConfigureAwait(false);
    await yield.ReturnAsync(123).ConfigureAwait(false);
})

On the other hand, I agree that doing synchronous calls on asynchronous methods just defies the whole purpose of proper use of the thread pool and IO port completion threads. So I'll consider the suggestion and keep this question open for now.

[TomPeters]

I understand the struggle with putting obnoxious ConfigureAwait(false), but this is the only right solution to make ASP.NET or WPF happy.

While I certainly get that putting ConfigureAwait(false) in the AsyncEnumerable enumerable library is a good idea, it is not the only right solution for people writing ASP.NET or WPF apps - the other solution is to use async all the way. If someone were to adopt that strategy in their own code, they might correctly omit the ConfigureAwait(false)s that you demonstrated in your example. And in this case, accidental synchronous enumeration of an AsyncEnumerable would necessarily deadlock :(.

The AsyncEnumerable internally always does Task.ConfigureAwait(false).GetAwaiter().Result on synchronous enumeration...

As a side note, I'm fairly sure that the ConfigureAwait(false) in this expression is redundant and does nothing (since there is no await to configure)

So I'll consider the suggestion and keep this question open for now.

👍

[kind-serge]

As a side note, I'm fairly sure that the ConfigureAwait(false) in this expression is redundant and does nothing (since there is no await to configure)

You are absolutely right. That affects only the task continuation: to use or not to use the SynchronizationContext for scheduling.

[kind-serge]

@TomPeters, this is a good suggestion, because thread blocking is just harmful and defeats the purpose of this library.
Now you need to explicitly do IAsyncEnumerable.ToEnumerable() for conversion.
The change is available in version 2.0.0.
Please see updated documentation section about the changes in version 2.

[TomPeters]

Looks good, thanks!