Skip to content

Fix over agressive async delegation#15521

Merged
rbuckton merged 3 commits into
masterfrom
fix15471
May 4, 2017
Merged

Fix over agressive async delegation#15521
rbuckton merged 3 commits into
masterfrom
fix15471

Conversation

@rbuckton
Copy link
Copy Markdown
Contributor

@rbuckton rbuckton commented May 2, 2017

When return() is called on a generator that is in the process of yield* delegation, it should not only call return() on the delegated iterable, but should also exit the current generator. The __asyncDelegator processes each call in two steps: first it indicates to the outer generator that it should await the result of a call to next, throw, or return, then it returns the result of that await operation in a tick/tock fashion.

When the delegator "ticks", it returns an IteratorResult that looks like: { value: ["await", result], done: false }. The outer async generator will handle the await and call back into the delegator ("tock"). When the delegator "tocks", it processes the iterator result and either exits with a done: true or yields with a { value: ["yield", value], done: false }.

The problem lies in the fact that when the delegator "ticks" as part of a call to return(), it returns done: false. This informs the underlying generator runtime that the delegator did not actually close, so it continues processing. This change now unconditionally causes the delegator to return by always returning done: true when calling return() on an async delegator.

This change also awaits the result of an async delegator (the x in x = yield* y) to more closely align with the async iteration proposal.

Fixes #15471

@rbuckton rbuckton requested a review from DanielRosenwasser May 2, 2017 20:07
@sandersn
Copy link
Copy Markdown
Member

sandersn commented May 3, 2017

I don't feel like I'm qualified to review this given my current knowledge of async implementation. Can you walk me through the PR in person and explain how everything works along the way?

Copy link
Copy Markdown
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the tutorial on async generators!

@rbuckton rbuckton merged commit e642691 into master May 4, 2017
@mhegazy
Copy link
Copy Markdown
Contributor

mhegazy commented May 4, 2017

@rbuckton can you port this change to release-2.3

@rbuckton
Copy link
Copy Markdown
Contributor Author

rbuckton commented May 4, 2017

@mhegazy this is now in release-2.3

@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Async generator is too eager

4 participants