fix(async-gen): async-generator await suspension (#6709)#6727
Merged
proggeramlug merged 5 commits intoJul 21, 2026
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAsync-generator lowering now models pending awaits as suspension states, resumes activations through shared boxed step closures, and settles results through a new runtime entry point. HIR, code generation, runtime queue ordering, and regression coverage were updated. ChangesAsync generator resume flow
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Consumer
participant Iterator
participant AsyncGeneratorStep
participant RuntimePromise
Consumer->>Iterator: Call next(value) or throw(error)
Iterator->>AsyncGeneratorStep: Resume shared step closure
AsyncGeneratorStep->>RuntimePromise: Suspend pending await
RuntimePromise-->>AsyncGeneratorStep: Resume with value or error
AsyncGeneratorStep-->>Iterator: Produce iterator result
Iterator-->>Consumer: Settle request promise
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/perry/tests/issue_6709_async_generator_pending_await.rs`:
- Around line 48-61: Update the module documentation describing “The fix (scoped
follow-up)” to past tense, stating that the async-generator await suspension fix
landed in this PR. Remove the claim that the test is #[ignore]d or awaiting a
future fix, while preserving the technical description of the implemented
lowering and AsyncStepChain behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 00eaca6a-1b09-4cd7-8a8d-49b27b7825b6
📒 Files selected for processing (16)
crates/perry-codegen/src/expr/dispatch.rscrates/perry-codegen/src/expr/misc_methods.rscrates/perry-codegen/src/runtime_decls/stdlib_ffi/language_core.rscrates/perry-hir/src/analysis/value_types.rscrates/perry-hir/src/ir/expr.rscrates/perry-hir/src/stable_hash/expr.rscrates/perry-hir/src/walker/expr_mut.rscrates/perry-hir/src/walker/expr_ref.rscrates/perry-runtime/src/object/async_generator_queue.rscrates/perry-runtime/src/promise/async_step.rscrates/perry-transform/src/async_to_generator.rscrates/perry-transform/src/generator/linearize.rscrates/perry-transform/src/generator/lower.rscrates/perry-transform/src/generator/lower/resume.rscrates/perry-transform/src/generator/mod.rscrates/perry/tests/issue_6709_async_generator_pending_await.rs
added 4 commits
July 20, 2026 17:10
…6709) Async generators busy-waited on `await` (the async_to_generator pass skipped generators, so `Expr::Await` nodes lowered to the poll loop) and deadlocked whenever the awaited Promise was resolved by an ancestor stack frame — the classic push/pull async iterator. This blocked pi's interactive TUI (its EventStream is an async generator). Make `.next()`/`.throw()` drive the state machine through the async-step machinery so inner `await`s suspend on the microtask queue: - linearize: split async-generator `await` points into a new `StateExit::Await`, distinct from consumer `Yield`; hoist non-top-level awaits first (reuse the plain-async `hoist_awaits_in_stmts`). - lower: `await` states `return AsyncStepChain(value, __step_self)`; yield/completion states settle the activation's result Promise with the `{value, done}` object (`AsyncStepDone`). `.next(v)`/`.throw(e)` are thin outer closures calling a shared `__agstep` step closure via the new `js_async_generator_resume` (like `js_async_first_call` but with an explicit value + is_error). `.return()` and sync generators keep the existing (busy-wait) dispatch. - runtime: add `js_async_generator_resume`; HIR `Expr::AsyncGenResume`. The runtime async_generator_queue already supported a pending `.next()` result Promise, so no queue changes are needed. Regression test issue_6709_async_generator_pending_await flipped from #[ignore]/deadlock to PASS.
…erryTS#6709) Follow-up to the async-generator await-suspension change. Now that `.next()` returns a *pending* Promise for every `yield` (the spec `AsyncGeneratorYield(? Await(value))` tick), the queue driver's pending path is always taken. It drained the next request BEFORE settling the current one, so a synchronously-completing follow-up (e.g. the terminal `{done:true}` after the last yield) resolved and fired its `.then` ahead of the earlier request — reordering the results of three eagerly-queued `iter.next()` calls (test262 request-queue-order*). Match the spec order (AsyncGeneratorResolve settles the front request's promise, THEN AsyncGeneratorDrainQueue resumes the next) by settling `out` before processing the next queued request. The immediate path already deferred the drain via schedule_drain, so only the pending path needed the swap. test262 async-generator parity: 947 -> 948 pass (2 previously-deadlocking request-queue-*-order tests now pass; one .return(thenable) tick-ordering edge case remains 1 microtask off — documented in the PR).
…note PerryTS#6728 keypress blocker (CodeRabbit on PerryTS#6727) Claude-Session: https://claude.ai/code/session_01JuiiePQfrXhAFD9fuCygB9
proggeramlug
force-pushed
the
fix/6709-async-gen-await-suspension
branch
from
July 20, 2026 15:11
f7793c0 to
6bcd7fe
Compare
Contributor
Author
|
Rebased onto current main (dropped the already-merged #6721 diagnosis commit) and addressed CodeRabbit: the regression-test module doc is now past-tense ('landed in this PR', test active/no #[ignore]), and I corrected the Impact section to note honestly that this async-gen fix does NOT by itself restore pi interactivity — an earlier keypress→submit divergence (#6728) is the current blocker; this fixes the async-generator deadlock in isolation. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/perry-transform/src/generator/lower.rs`:
- Around line 103-121: Update the StateExit::Await arm in the surrounding
lowering logic to apply prepend_done_before_returns and rewrite_returns_as_done
to case_body before emitting the await transition, matching the return handling
used by StateExit::Yield, Goto, and Done. Ensure nested user returns are
converted to AsyncStepDone completion rather than escaping as raw returns, while
preserving the existing async_step and busy-wait behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 74e5637b-04bd-4429-8cd6-f1bb2743a7bc
📒 Files selected for processing (16)
crates/perry-codegen/src/expr/dispatch.rscrates/perry-codegen/src/expr/misc_methods.rscrates/perry-codegen/src/runtime_decls/stdlib_ffi/language_core.rscrates/perry-hir/src/analysis/value_types.rscrates/perry-hir/src/ir/expr.rscrates/perry-hir/src/stable_hash/expr.rscrates/perry-hir/src/walker/expr_mut.rscrates/perry-hir/src/walker/expr_ref.rscrates/perry-runtime/src/object/async_generator_queue.rscrates/perry-runtime/src/promise/async_step.rscrates/perry-transform/src/async_to_generator.rscrates/perry-transform/src/generator/linearize.rscrates/perry-transform/src/generator/lower.rscrates/perry-transform/src/generator/lower/resume.rscrates/perry-transform/src/generator/mod.rscrates/perry/tests/issue_6709_async_generator_pending_await.rs
🚧 Files skipped from review as they are similar to previous changes (14)
- crates/perry-transform/src/generator/mod.rs
- crates/perry-codegen/src/runtime_decls/stdlib_ffi/language_core.rs
- crates/perry-hir/src/walker/expr_mut.rs
- crates/perry-hir/src/ir/expr.rs
- crates/perry-hir/src/stable_hash/expr.rs
- crates/perry-transform/src/generator/lower/resume.rs
- crates/perry-hir/src/walker/expr_ref.rs
- crates/perry-runtime/src/promise/async_step.rs
- crates/perry-codegen/src/expr/dispatch.rs
- crates/perry-codegen/src/expr/misc_methods.rs
- crates/perry-transform/src/async_to_generator.rs
- crates/perry-hir/src/analysis/value_types.rs
- crates/perry-transform/src/generator/linearize.rs
- crates/perry/tests/issue_6709_async_generator_pending_await.rs
…#6709) The `StateExit::Await` arm of `build_dispatch_while_body` skipped the `prepend_done_before_returns` + `rewrite_returns_as_done` pass that the Yield/Goto/Done arms apply. A user `return X` in a yield/await-free control-flow block (e.g. `if (c) return X;`) is accumulated by the linearizer's catch-all into the Await state's body ahead of the `await`, so it reached this arm as a raw return. `wrap_async_gen_step_body` does not wrap step-body returns in `Promise.resolve` (its invariant is that every return is already `AsyncStepChain`/`AsyncStepDone`/`Promise.reject`), and `wrap_iter_result_returns_in_async_step_done` only touches iter-result objects — so the raw `return X` escaped: `.next()` resolved to the bare value `X` instead of `{value: X, done: true}`, and (done flag never set) the next `.next()` re-ran the state. Mirror the return rewrite in the Await arm, gated on `body_contains_return` so the common no-return path is unchanged. Regression test added; verified byte-identical to node before/after and across an adjacent-shape battery. CodeRabbit on PerryTS#6727. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #6709. Follow-up to the diagnosis in #6721 (merged): implements the async-generator
await-suspension fix.What was broken
async function*bodies did not suspend onawait— they busy-waited (theasync_to_generatorpass skips generators, soExpr::Awaitnodes lowered to the poll loop infs_await.rs), and deadlocked whenever the awaited Promise was resolved by an ancestor stack frame (the classic push/pull async iterator).it.next()on an async generator awaiting a pending Promise never returned.Implementation
Make
.next()/.throw()drive the state machine through the existing async-step machinery so innerawaits suspend on the microtask queue:generator/linearize.rs): split async-generatorawaitpoints into a newStateExit::Await, distinct from consumerYield. Non-top-level awaits are hoisted first (reuse of the plain-asynchoist_awaits_in_stmts).generator/lower.rs):awaitstatesreturn AsyncStepChain(value, __step_self)(suspend on the microtask queue); consumer-yield/completion states settle the activation's result Promise with the{value, done}object viaAsyncStepDone..next(v)/.throw(e)are thin outer closures that call a shared__agstepstep closure via the newjs_async_generator_resume(likejs_async_first_call, but with an explicit value +is_error). Anawaitthat rejects re-enters the step withis_error=true, routing to the enclosing catch exactly like an explicit.throw()..return()and sync generators keep the existing (busy-wait) dispatch.promise/async_step.rs): addjs_async_generator_resume. HIR:Expr::AsyncGenResume.object/async_generator_queue.rs): now that.next()returns a pending Promise for everyyield(specAsyncGeneratorYield(? Await(value))), the queue driver's pending path is always taken; settle the current request's promise before draining the next (spec order:AsyncGeneratorResolvethenAsyncGeneratorDrainQueue), else a synchronously-completing follow-up (the terminal{done:true}) reordered results.The runtime
async_generator_queuealready supported a pending.next()result Promise, so no other queue changes were needed.Validation
crates/perry/tests/issue_6709_async_generator_pending_await.rs(the exact EventStream push/pull shape): flipped from#[ignore]/deadlock → PASS.language/{statements,expressions}/async-generator,built-ins/AsyncGenerator{Function,Prototype},language/statements/for-await-of):request-queue-*-ordertests now pass; −1 (yield-return-then-getter-ticks.js). The 7 runtime-fails + 29 still-failing are pre-existing gaps unrelated to await-suspension (AsyncGeneratorFunction dynamic construction, await/yield-in-params negative tests)..return(thenable)microtask tick-ordering artifact: node fires the.thengetter between ticks 1–2; perry, one microtask later. No wrong result and no deadlock — the head/queued request is drained via a promise settle-listener (1 tick after the step resolves it) rather than node's synchronous drain. Cheaply fixable only with an architectural change coupling the step driver to the queue; accepted as-is (invisible to real code — only a conformance probe with a side-effecting.thengetter observes it).perry-transform53/53;perry-codegen208 pass (+1 pre-existing failure that also fails at the diagnosis commitdafd067c);perry-runtime --test-threads=11429 pass (+1 pre-existing GC/JSON failure, also fails atdafd067c).yield*to an async iterable with inner awaits, two-waynext,.throwinto a catch that yields,.return+finally, plain async fns still work); an edge battery (async-gen expressions, nested async gens, realsetTimeoutawaits).pi interactive: honest result
The change is confirmed compiled into a fresh pi build (
/tmp/pi-fixed): the auto-optimize runtime archive it links containsjs_async_generator_resume, and pi'sEventStream[Symbol.asyncIterator]is the exact push/pull shape the regression fixture covers (which works in isolation).However, pi's interactive Enter-submit is byte-identical before vs. after (the issue's PTY repro: type
hello, CR0x0d, bogus key). With the fix, CR Enter still leaveshellostuck in the input — no "Working…", no 401 — same as the unfixed binary. Node renders the 401. So the async-generator deadlock is genuinely fixed, but it is not what blocks pi's Enter-submit — pressing Enter (CR) never fires the submit (the input isn't cleared), so the agent loop / EventStream never runs. Sending LF (0x0a) instead produces different output (input clears), pointing to a keypress→submit / raw-stdin decoding divergence (issue hypothesis #1), separate from the async-generator event-forwarding the diagnosis attributed. That remaining pi blocker is out of scope for this change and needs its own investigation.Do NOT merge without a maintainer's call on the pi attribution + the one tick-ordering tradeoff.
Summary by CodeRabbit
New Features
.next(value)and.throw(error)handling so resumes route the caller-supplied value/error through the generator step correctly.awaitsuspension handling forawait x;,let v = await x;,return await x;, andthrow await x;.Bug Fixes
Tests