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

Champion "Async Streams" (including async disposable) (16.3, Core 3) #43

Open
gafter opened this issue Feb 9, 2017 · 64 comments
Open

Champion "Async Streams" (including async disposable) (16.3, Core 3) #43

gafter opened this issue Feb 9, 2017 · 64 comments
Assignees
Milestone

Comments

@gafter
Copy link
Member

@gafter gafter commented Feb 9, 2017

See also dotnet/roslyn#261

LDM notes:

@gafter

This comment has been minimized.

Copy link
Member Author

@gafter gafter commented Feb 15, 2017

This would be something like

interface IAsyncEnumerable<T>
{
        IAsyncEnumerator<T> GetEnumerator();
}
interface IAsyncEnumerator<T>: IAsyncDisposable
{
        Task<bool> MoveNextAsync();
        T Current { get; }
}

With the following elements yet to be designed

  • IAsyncDisposable
  • async foreach loops
  • Declaring async iterator methods
  • Some kind of query expression support
@gafter gafter changed the title Champion "Async Streams" Champion "Async Streams" (including async disposable) Feb 15, 2017
@gafter gafter added this to the 8.0 candidate milestone Feb 22, 2017
@onovotny

This comment has been minimized.

Copy link
Member

@onovotny onovotny commented Feb 23, 2017

@gafter I thought we settled on

interface IAsyncEnumerable<T>
{
        IAsyncEnumerator<T> GetEnumerator(CancellationToken cancellation);
}

And potentially

static class AsyncEnumerableExtensions
{
    static IAsyncEnumerator<T> GetEnumerator(this IAsyncEnumerable<T> source) => source.GetEnumerator(CancellationToken.None);
}
@juepiezhongren

This comment has been minimized.

Copy link

@juepiezhongren juepiezhongren commented May 11, 2017

After the appearance of "IAsyncEnumerable" in Azure Service Fabric, we want more.........
We want the entire async implementation of .net collection BCL part!!!!!!
AsyncLinq: SelectAsync... AsyncParalellLinq: AsAsyncParalell... await yield!!!!!

As ecmascript is proposing with generator async, there is to be the fulfilment in .net!
And with what i have mentioned being implemented, .net(C# and F#) is going to be the most developer-friendly language for big-data, and service-fabric is gonna boosting a completely new ecosystem with a firm asynchronous programming foundation.

wish these come true earlier!!!!

@oliverjanik

This comment has been minimized.

Copy link

@oliverjanik oliverjanik commented Jun 2, 2017

Any news on this? Seems this interface has been defined in several places already (IX, EF7)

@benaadams

This comment has been minimized.

Copy link
Member

@benaadams benaadams commented Aug 22, 2017

As brought up in roslyn thread

interface IAsyncEnumerator<T>: IAsyncDisposable
{
    Task<bool> MoveNextAsync();
    T Current { get; }
}

Has issues with parallelazation/concurrency/atomicity which is a desirable property of an async system (e.g. double gets on Current or missed items on Current)

@IvanKonov

This comment has been minimized.

Copy link

@IvanKonov IvanKonov commented Aug 22, 2017

Maybe then:

interface IAsyncEnumerator<T> : IAsyncDisposable
{
    Task<bool> GetNextAsync(out T next);;
    Task<T> Current { get; }
}
@HaloFour

This comment has been minimized.

Copy link
Contributor

@HaloFour HaloFour commented Aug 22, 2017

An async method can't have out parameters.

@IvanKonov

This comment has been minimized.

Copy link

@IvanKonov IvanKonov commented Aug 22, 2017

Another version:

interface IAsyncEnumerator<T> : IAsyncDisposable
{
    Task<(bool has_value, T next)> GetNextAsync();
    Task<T> Current { get; }
}
@onovotny

This comment has been minimized.

Copy link
Member

@onovotny onovotny commented Aug 22, 2017

How about just adding out to async methods? That would make this far cleaner :trollface:

@vladd

This comment has been minimized.

Copy link

@vladd vladd commented Aug 22, 2017

@IvanKonov This definition would make IAsyncEnumerator<T> not covariant by T.

@benaadams

This comment has been minimized.

Copy link
Member

@benaadams benaadams commented Aug 22, 2017

An async method can't have out parameters.

but

Task<bool> GetNextAsync(out T next);

isn't async; its just Task<bool> returning, and you can await with out params so no problems? 😉

async Task DoThing()
{
    var enumerator = new Enumerator<int>();
    while(await enumerator.GetNextAsync(out var next))
    {
        // Do thing with next
    }
}

struct Enumerator<T>
{
    public Task<bool> GetNextAsync(out T next)
    {
        next = default(T);
        return Task.FromResult(false);
    }
}
@yaakov-h

This comment has been minimized.

Copy link
Contributor

@yaakov-h yaakov-h commented Aug 22, 2017

Doesn't the out param have to be set before the task is returned? At that point, the task will most likely not be completed, so you won't have a "next" to pass as the out param. Then, by the time you have your next, it's too late to fill the out param.

@benaadams

This comment has been minimized.

Copy link
Member

@benaadams benaadams commented Aug 22, 2017

Doesn't the out param have to be set before the task is returned?

😢 I'll go with @onovotny's suggestion instead then #43 (comment)

@stephentoub

This comment has been minimized.

Copy link
Member

@stephentoub stephentoub commented Aug 22, 2017

Has issues with parallelazation/concurrency/atomicity which is a desirable property of an async system

While true, if you have a source that supports multiple readers, you can also simply change the model to one where each reader is given its own enumerator. Then the concurrency issues evaporate / are left up to the coordinator that's giving the items out to the individual enumerators.

I'll go with @onovotny's suggestion instead

I don't understand the suggestion. When I do:

Task<bool> t = enumerator.GetMoveNext(out T result);
Use(result);
await t;

how is that intended to work?

covariant

Note that most of this was already discussed in dotnet/roslyn#261.

@onovotny

This comment has been minimized.

Copy link
Member

@onovotny onovotny commented Aug 22, 2017

I was trolling, but what if using out parameters with Task required use of await? Then the assignment order is guaranteed.

@stephentoub

This comment has been minimized.

Copy link
Member

@stephentoub stephentoub commented Aug 22, 2017

I was trolling, but what if using out parameters with Task required use of await? Then the assignment order is guaranteed.

a) that would be a breaking change, b) there are actually scenarios where you want to access the out value after the synchronous return of the method, and c) anything can be made awaitable, so task isn't special in this regard.

@yaakov-h

This comment has been minimized.

Copy link
Contributor

@yaakov-h yaakov-h commented Aug 22, 2017

@stephentoub is there a Cliffs Notes version of that thread?

@stephentoub

This comment has been minimized.

Copy link
Member

@stephentoub stephentoub commented Aug 22, 2017

In the near future I'm planning to take some time to write down where we are, which would include summarizing salient points from that thread.

@scottt732

This comment has been minimized.

Copy link

@scottt732 scottt732 commented Aug 22, 2017

Have you flushed out how CancellationTokens would work here? Is the intent to explicitly call GetEnumerator... something like foreach (await var item in ae.GetEnumerator(cancellationToken)) { ... } ?

Task<bool> MoveNextAsync(CancellationToken cancellationToken) in the interface feels more consistent, but I gather that would require additional language changes to accommodate in foreach/while, LINQ query/method syntax.

Would it be possible to get the IEnumerable/LINQ and IObservable/Rx extension methods wired up to IAsyncEnumerable? Do IAsyncQueryable/IAsyncQbservable factor in to the plans at all?

@Igorbek

This comment has been minimized.

Copy link

@Igorbek Igorbek commented Aug 23, 2017

discriminated unions would be useful here.

@NetMage

This comment has been minimized.

Copy link

@NetMage NetMage commented Jan 12, 2018

Personally I would really prefer flipping TryGetNext around:

Task WaitForNextAsync();
bool TryGetNext(out T nextVal);

@benaadams

This comment has been minimized.

Copy link
Member

@benaadams benaadams commented Jan 12, 2018

Personally I would really prefer flipping TryGetNext around:

Discarded options considered:

Task<bool> WaitForNextAsync(); bool TryGetNext(out T result);: out parameters can't be covariant. There's also a small impact here (an issue with the try pattern in general) that this likely incurs a runtime write barrier for reference type results.

Probably should be When not Wait? As When is normally used in preference to Wait for async?

Task<bool> WhenNextAsync();
@bbarry

This comment has been minimized.

Copy link
Contributor

@bbarry bbarry commented Jan 12, 2018

I like the "viable alternative"

namespace System.Collections.Generic
{
    public interface IAsyncEnumerable<out T>
    {
        IAsyncEnumerator<T> GetAsyncEnumerator();
    }

    public interface IAsyncEnumerator<out T> : IAsyncDisposable
    {
        Task<bool> WaitForNextAsync();
        T TryGetNext(out bool success);
    }
}

Is it being considered to implement a method pattern version instead of explicitly settling on specific interfaces:

TAsyncEnumerable<out TItem>
{
    TAsyncEnumerator<TItem> GetAsyncEnumerator();
}

TAsyncEnumerator<out TItem>
{
    TTaskLike<bool> WaitForNextAsync();
    T TryGetNext(out bool success);
    TTaskLike DisposeAsync();
}
@alrz

This comment has been minimized.

Copy link
Contributor

@alrz alrz commented Jan 17, 2018

Currently, using statements swallow try block exception if Dispose also throws and async methods do not aggregate exceptions even though they throw AggregateException. Are we going to change that behavior for using await or DisposeAsync is supposed to be error free? I suspect async methods are more likely to fail since there are more moving parts involved, so it could be harder to maintain a safe implementation to take advantage of using await and not fallback to a nested try finally as a precaution for critical code.


Another question for async streams, what if the iteration itself is not async but I have an await somewhere?

async IAsyncEnumerable<T> IteratorAsync() {
  using (var reader = await ExecuteReader()) {
    while (reader.Read())
      yield return RowParser(reader);
  }
}

Currently I can accomplish that with Task<IEnumerable<T>> and a iterator local function. MoveNextAsync for this example is unnecessary. Do we allow Task<IEnumerable<T>> as the return type in these situations?

@quinmars

This comment has been minimized.

Copy link

@quinmars quinmars commented Jan 19, 2018

@alrz I guess that would be very tricky. Who will dispose the reader? I think that will only work with buffering of the complete list or with IAsyncEnumerable<T>. The first MoveNextAsync will take sometime because the reader needs to be awaited, the following MoveNextAsync will be fast. For them, the state machine can simply return Task.FromResult(true) or Task.FromResult(false) or even better a cached or global reference of them.

@alrz

This comment has been minimized.

Copy link
Contributor

@alrz alrz commented Jan 19, 2018

RE "Who will dispose the reader" The iterator.

async Task<IEnumerable<T>> IteratorAsync() {
  var reader = await ExecuteReader();
  IEnumerable<T> Iterator() {
    using (reader)
      while (reader.Read())
        yield return RowParser(reader);
  }
  return Iterator();
}

It's not always easy to workaround this, would be nice if the compiler can see if the iterator is not awaiting and permit Task<IEnumerable<T>> as the return type.

@quinmars

This comment has been minimized.

Copy link

@quinmars quinmars commented Jan 19, 2018

@alrz I see. But it's rather unnoticeable, that you will leak resources if you do not use the IEnumerable<T> once.

@bbarry

This comment has been minimized.

Copy link
Contributor

@bbarry bbarry commented Jan 21, 2018

If the alternative interfaces are used, I don't think there is any benefit to permitting Task<IEnumerable<T>> (maybe 1 virtual call? If the pattern form was used they could potentially even be inlineable).

@TylerBrinkley

This comment has been minimized.

Copy link

@TylerBrinkley TylerBrinkley commented May 16, 2018

I was watching the Build demo of this feature and was wondering how we can specify .ConfigureAwait(false) for async disposal and async enumeration?

@TylerBrinkley

This comment has been minimized.

Copy link

@TylerBrinkley TylerBrinkley commented May 16, 2018

Nevermind, it appears this is being considered in the proposal.

@svick

This comment has been minimized.

Copy link
Contributor

@svick svick commented May 24, 2018

From May 21 2018 LDM notes:

foreach await over dynamic

Block it. For synchronous foreach we resort to the nongeneric IEnumerable, but there is no nongeneric IAsyncEnumerable, and there won't be.

Since the IAsyncEnumerable interface is going to be covariant, would it make sense if foreach await over dynamic worked with IAsyncEnumerable<object>? It would work only for async enumerables of reference types, but maybe that's better than nothing?

@Neme12

This comment has been minimized.

Copy link

@Neme12 Neme12 commented Sep 27, 2018

How will async disposables interact with #1174? Will there be an async form of that?

@masonwheeler

This comment has been minimized.

Copy link

@masonwheeler masonwheeler commented Feb 27, 2019

This link at the top of the document returns a 404

@stephentoub

This comment has been minimized.

Copy link
Member

@stephentoub stephentoub commented Feb 27, 2019

Fixed. It moved into the C# 8 folder.

@gafter gafter added this to 8.0 Candidate in Language Version Planning Mar 6, 2019
@jcouv jcouv changed the title Champion "Async Streams" (including async disposable) Champion "Async Streams" (including async disposable) (16.3, Core 3) Jul 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
You can’t perform that action at this time.