Skip to content

Commit 80fb6c5

Browse files
author
Ralph Küpper
committed
fix(async): linearize await inside an async-generator finally; fix aliased native-class new
Lands #8736 and #8738. #8736 (fixes #8715) closes the `finally` analog of the #8681 `await`-in-`catch` deadlock that #8707 fixed. This is the exact gap #8707's own new test surfaced when it was rebased -- it reported "await-in-finally: 2 raw await(s) survived" -- so the two land as a pair. An `await` inside a `finally` of a real `async function*` compiled to a blocking busy-wait rather than an async suspend; the linearizer already splits the finally into its own dispatch states with a `finally_entry_state`, and the async-step driver now routes through them. #8738 (fixes #8730) stops an aliased ESM named import of a Node built-in class throwing `ReferenceError: identifier is not defined` when constructed at module init -- `import { BlockList as Wj4 } from "net"; new Wj4()` and the same shape for `AsyncLocalStorage` and `PassThrough`. `lower_new`'s alias-rewrite block rewrites the callee from the local import name to the class's export name so the construction path matches the un-aliased form that codegen's builtin-`New` dispatch recognizes. This broke the natively-compiled Claude Code cli.js 2.1.112 bundle, which constructs all three at module init, so nearly every command crashed. No version bump.
1 parent c203c77 commit 80fb6c5

6 files changed

Lines changed: 224 additions & 25 deletions

File tree

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
fix(async): an `await` inside a `finally` of an `async function*` no longer
2+
compiles to a blocking busy-wait. When a `try` in an async generator has a
3+
finally that yields or awaits, the finally is linearized into its own dispatch
4+
states, and `.next()`/`.throw()` drive those states through the shared async-step
5+
driver so their `await`s suspend on the microtask queue. The `.return()` closure,
6+
however, re-drove the same states through a separate busy-wait dispatch loop
7+
(`__sent = await value; continue`) — so a `.return()` that ran the finally (an
8+
early `break` in a `for await`, or an explicit `.return()`) block-waited on the
9+
finally's `await`, monopolising the single runtime thread and deadlocking. This
10+
is the finally analog of the #8681 `await`-in-`catch` deadlock.
11+
12+
`.return()` now hands the continuation off to the shared `__agstep` driver
13+
(a fresh non-error resume) after routing the pending return into the finally,
14+
exactly as `.next()`/`.throw()` already do, so a finally `await` suspends
15+
instead of blocking. Behavior for a finally that only yields is unchanged.
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix an aliased ESM named import of a Node built-in class (`import { BlockList as Wj4 } from "net"`, `{ AsyncLocalStorage as J_z } from "async_hooks"`, `{ PassThrough as Lrz } from "stream"`) throwing `ReferenceError: identifier is not defined` when constructed. The `new`-lowering already rewrites the alias to the class's export name so construction matches the un-aliased form, but the unresolved-`new` guard added in #8688 re-checked the native-module registry under that rewritten export name — which is keyed on the local import name — and, since these classes are not reified global builtins, fired the nameless throw at module init. The guard now also consults the registry under the original imported identifier, so aliased native-class imports resolve and construct exactly like their un-aliased form. This unblocked the natively compiled Claude Code cli.js 2.1.112 bundle, which crashed at module init on nearly every command.

crates/perry-hir/src/lower/expr_new.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1571,12 +1571,26 @@ pub(super) fn lower_new(ctx: &mut LoweringContext, new_expr: &ast::NewExpr) -> R
15711571
// evaluating the constructor reference. That is a ReferenceError
15721572
// (`new Missing()`), distinct from the TypeError produced when a
15731573
// present binding's value is non-constructable.
1574+
//
1575+
// Consult the native-module registry under BOTH the (possibly
1576+
// rewritten) `class_name` AND the original `source_class_name`.
1577+
// The alias-rewrite block just above replaces `class_name` with a
1578+
// native class's EXPORT name (`Wj4` → `BlockList`) so the
1579+
// construction path below matches the un-aliased form, but the
1580+
// registry is keyed on the LOCAL import name (`Wj4`), so
1581+
// `lookup_native_module(&class_name)` misses under the export name.
1582+
// Checking `source_class_name` recognizes the aliased native import
1583+
// as resolved; without it, an aliased `import { BlockList as Wj4 }`
1584+
// / `{ AsyncLocalStorage as J_z }` / `{ PassThrough as Lrz }` (none
1585+
// of which are reified global builtins) fell through to this throw
1586+
// at module init even though the binding is perfectly resolvable.
15741587
if ctx.lookup_class(&class_name).is_none()
15751588
&& ctx.resolve_class_alias(&class_name).is_none()
15761589
&& ctx.lookup_local(&class_name).is_none()
15771590
&& ctx.lookup_func(&class_name).is_none()
15781591
&& ctx.lookup_imported_func(&class_name).is_none()
15791592
&& ctx.lookup_native_module(&class_name).is_none()
1593+
&& ctx.lookup_native_module(source_class_name).is_none()
15801594
&& !ctx.forward_class_names.contains(source_class_name)
15811595
&& !is_reified_global_builtin_constructor(&class_name)
15821596
{
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
//! Regression test for #8730: an ALIASED ESM named import of a Node built-in
2+
//! class (`import { BlockList as Wj4 } from "net"; new Wj4()`) must not lower
3+
//! `new <alias>()` to the nameless `js_throw_reference_error_unresolved_get`
4+
//! throw.
5+
//!
6+
//! Root cause: the alias-rewrite block in `lower_new` replaces the callee's
7+
//! `class_name` with the native class's EXPORT name (`Wj4` -> `BlockList`) so
8+
//! the construction path matches the un-aliased form, but the freshly-added
9+
//! (#8688) unresolved-`new` guard then consulted `lookup_native_module` under
10+
//! that rewritten export name — which is not in the registry (it is keyed on
11+
//! the LOCAL import name). None of these classes are reified global builtins,
12+
//! so the guard fired and every command threw `ReferenceError: identifier is
13+
//! not defined` at module init.
14+
15+
use perry_diagnostics::SourceCache;
16+
use perry_hir::lower_module;
17+
use perry_parser::parse_typescript_with_cache;
18+
19+
const THROW_HELPER: &str = "js_throw_reference_error_unresolved_get";
20+
21+
fn lower_debug(src: &str) -> String {
22+
let src = src.to_string();
23+
std::thread::Builder::new()
24+
.stack_size(32 * 1024 * 1024)
25+
.spawn(move || {
26+
let mut cache = SourceCache::new();
27+
let parsed =
28+
parse_typescript_with_cache(&src, "aliased_native_new_resolution.ts", &mut cache)
29+
.expect("parse should succeed");
30+
let module = lower_module(&parsed.module, "test", "aliased_native_new_resolution.ts")
31+
.expect("lowering should succeed");
32+
format!("{module:#?}")
33+
})
34+
.expect("spawn lower thread")
35+
.join()
36+
.expect("lower thread panicked")
37+
}
38+
39+
#[test]
40+
fn aliased_native_class_import_does_not_lower_to_nameless_throw() {
41+
// Each mirrors a real cli.js 2.1.112 shape from #8730 (BlockList/Wj4 built
42+
// and `.addSubnet`-ed at module init; AsyncLocalStorage/J_z; PassThrough).
43+
let cases = [
44+
(
45+
"BlockList",
46+
r#"import { BlockList as Wj4 } from "net";
47+
const b = new Wj4();
48+
b.addSubnet("10.0.0.0", 8);
49+
console.log(b.check("10.1.2.3"));"#,
50+
),
51+
(
52+
"AsyncLocalStorage",
53+
r#"import { AsyncLocalStorage as J_z } from "async_hooks";
54+
const s = new J_z();
55+
console.log(typeof s.run);"#,
56+
),
57+
(
58+
"PassThrough",
59+
r#"import { PassThrough as Lrz } from "stream";
60+
const p = new Lrz();
61+
console.log(typeof p.pipe);"#,
62+
),
63+
];
64+
65+
for (label, src) in cases {
66+
let debug = lower_debug(src);
67+
assert!(
68+
!debug.contains(THROW_HELPER),
69+
"aliased native import `{label}` must construct, not throw the nameless \
70+
ReferenceError at module init:\n{debug}"
71+
);
72+
}
73+
}
74+
75+
#[test]
76+
fn unaliased_native_class_import_still_constructs() {
77+
// Control: the un-aliased form was never broken; keep it green so the fix
78+
// is symmetric across aliased/un-aliased native imports.
79+
let debug = lower_debug(
80+
r#"import { BlockList } from "net";
81+
const b = new BlockList();
82+
b.addSubnet("10.0.0.0", 8);
83+
console.log(b.check("10.1.2.3"));"#,
84+
);
85+
assert!(
86+
!debug.contains(THROW_HELPER),
87+
"un-aliased native import must construct, not throw:\n{debug}"
88+
);
89+
}
90+
91+
#[test]
92+
fn genuinely_unresolved_new_still_throws() {
93+
// Positive control: the guard must still fire for a `new` on an identifier
94+
// that resolves to no binding at all — the fix must not blanket-suppress it.
95+
let debug = lower_debug(r#"const x = new Totally_Undefined_Constructor_Xyz();"#);
96+
assert!(
97+
debug.contains(THROW_HELPER),
98+
"a genuinely unresolved `new` must still lower to the nameless \
99+
ReferenceError throw:\n{debug}"
100+
);
101+
}

crates/perry-transform/src/async_to_generator_tests.rs

Lines changed: 43 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -576,16 +576,49 @@ fn async_generator_linearizes_every_await_position() {
576576
finally: None,
577577
}],
578578
),
579-
// NOTE: `await` inside a `finally` of a REAL async generator
580-
// (`async function*`) is a SEPARATE, pre-existing gap in the
581-
// `#4438` B2-finally lowering — the yielding finally's states are
582-
// built with a raw `Expr::Await` instead of an async suspend, so it
583-
// block-waits the same way. It is NOT addressed by this PR (which
584-
// fixes the `was_plain_async` catch path); the closure test
585-
// `async_closure_rewrite_leaves_no_residual_await` DOES cover
586-
// `in-finally` for the `was_plain_async` path, which is clean.
587-
// Tracked separately in #8715; omitted here so this test asserts
588-
// only what this change fixes.
579+
// #8715: `await` inside a `finally` of a REAL async generator
580+
// (`async function*`). The yielding finally is linearized into its own
581+
// dispatch states, but the `.return()` closure used to re-drive them
582+
// through an async_step=false busy-wait loop (`__sent = await v;
583+
// continue`) — a blocking wait, the finally analog of the #8681 catch
584+
// deadlock. `.return()` now delegates the continuation to the shared
585+
// `__agstep` driver, so the finally `await` suspends on the microtask
586+
// queue and no raw `Expr::Await` survives.
587+
(
588+
"await-in-finally",
589+
vec![Stmt::Try {
590+
body: vec![y(Expr::Integer(0))],
591+
catch: None,
592+
finally: Some(vec![Stmt::Expr(await_(Expr::Integer(1)))]),
593+
}],
594+
),
595+
(
596+
"await-in-try-and-finally",
597+
vec![Stmt::Try {
598+
body: vec![Stmt::Expr(await_(Expr::Integer(0))), y(Expr::Integer(5))],
599+
catch: None,
600+
finally: Some(vec![Stmt::Expr(await_(Expr::Integer(1)))]),
601+
}],
602+
),
603+
(
604+
"await-in-try-catch-finally",
605+
vec![Stmt::Try {
606+
body: vec![y(Expr::Integer(0))],
607+
catch: Some(CatchClause {
608+
param: None,
609+
body: vec![Stmt::Expr(await_(Expr::Integer(1)))],
610+
}),
611+
finally: Some(vec![Stmt::Expr(await_(Expr::Integer(2)))]),
612+
}],
613+
),
614+
(
615+
"yield-in-finally-with-await",
616+
vec![Stmt::Try {
617+
body: vec![y(Expr::Integer(0))],
618+
catch: None,
619+
finally: Some(vec![y(Expr::Integer(8)), Stmt::Expr(await_(Expr::Integer(9)))]),
620+
}],
621+
),
589622
(
590623
"await-in-if-inside-try-inside-loop",
591624
// The pi #6728 shape: await buried in nested control flow.

crates/perry-transform/src/generator/lower.rs

Lines changed: 50 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -516,13 +516,19 @@ pub fn transform_generator_function_with_extra_captures(
516516
// #4374: clone the state-dispatch loop so the .throw() closure can
517517
// *continue* the state machine after running a catch handler.
518518
let while_body_for_throw = while_body.clone();
519-
// #4438 B2-finally: the `.return()` closure needs the same continuation loop
520-
// when it routes into a yielding finally (so the finally's `yield`s suspend).
521-
// #6709: the `.return()` closure is NOT an async-step driver (it cannot
522-
// chain an inner `await` through `CurrentStepClosure`), so its dispatch
523-
// keeps the busy-wait `await` shape — matching pre-#6709 `.return()`.
519+
// #4438 B2-finally: a `.return()` that routes into a yielding finally must
520+
// keep driving the state machine so the finally's `yield`s/`await`s run.
521+
// #8715: async generators delegate that continuation to the shared `__agstep`
522+
// step driver (see the `has_yielding_finally` branch below), so an `await`
523+
// inside the finally suspends on the microtask queue via `AsyncStepChain`
524+
// exactly as it does on the `.next()`/`.throw()` paths. Building an
525+
// async_step=false dispatch loop here instead would lower every such `await`
526+
// to a blocking busy-wait (`__sent = await v; continue`) — the finally analog
527+
// of the #8681 catch deadlock — so async generators build none. Sync
528+
// generators keep the busy-wait clone: they have no `await` states, so it
529+
// stays correct, and their `.return()` is a plain (non-driver) closure.
524530
let while_body_for_return = if is_async_generator {
525-
build_dispatch_while_body(&states, false, state_id, done_id, sent_id)
531+
Vec::new()
526532
} else {
527533
while_body.clone()
528534
};
@@ -577,7 +583,10 @@ pub fn transform_generator_function_with_extra_captures(
577583
} else {
578584
while_body_for_throw
579585
};
580-
let while_body_for_return = if wrap_dispatch {
586+
// #8715: async generators no longer run a local `.return()` dispatch loop
587+
// (`while_body_for_return` is empty — they delegate to `__agstep`), so skip
588+
// wrapping it. Sync generators still wrap their busy-wait clone.
589+
let while_body_for_return = if wrap_dispatch && !is_async_generator {
581590
let disp_err_id = alloc_local(next_local_id);
582591
wrap_dispatch_loop(
583592
while_body_for_return,
@@ -977,10 +986,10 @@ pub fn transform_generator_function_with_extra_captures(
977986
))));
978987
if has_yielding_finally {
979988
// #4438 B2-finally: route `.return(v)` into the innermost enclosing
980-
// yielding finally (record the pending return + jump in), then fall
981-
// through to the continuation loop so the finally's `yield`s suspend;
982-
// its completion check re-raises the return. Catches don't catch a
983-
// return completion, so only finally routes apply.
989+
// yielding finally record the pending return and jump to
990+
// `finally_entry_state`. Catches don't catch a return completion, so
991+
// only finally routes apply; on no match, `return_fallback` completes
992+
// the generator directly (never reaching the continuation below).
984993
return_resume_body.extend(build_abrupt_routing(
985994
&catches,
986995
&finallys,
@@ -994,10 +1003,36 @@ pub fn transform_generator_function_with_extra_captures(
9941003
false,
9951004
return_fallback,
9961005
));
997-
return_resume_body.push(Stmt::While {
998-
condition: Expr::Bool(true),
999-
body: while_body_for_return,
1000-
});
1006+
if is_async_generator {
1007+
// #8715: a matched route has set `state = finally_entry_state`
1008+
// and recorded the pending return in the shared boxed locals.
1009+
// Hand off to the shared `__agstep` driver (a fresh, non-error
1010+
// resume) rather than run a local async_step=false loop, so a
1011+
// finally `await` suspends on the microtask queue (`AsyncStepChain`
1012+
// re-entering `__agstep`) instead of block-waiting — the fix for
1013+
// this issue. `__agstep` dispatches from `finally_entry_state`,
1014+
// runs the finally (its `yield`s settle this `.return()`'s
1015+
// promise, its `await`s suspend), and its completion-check state
1016+
// re-raises the pending return as `{value, done: true}`. This
1017+
// mirrors how `.next()`/`.throw()` already drive a yielding
1018+
// finally. `wrap_generator_resume_body` clears `executing` before
1019+
// this return, so `__agstep`'s re-entrancy guard passes.
1020+
let agstep_local_id =
1021+
agstep_id.expect("agstep_id is set for async generators");
1022+
return_resume_body.push(Stmt::Return(Some(Expr::AsyncGenResume {
1023+
step_closure: Box::new(Expr::LocalGet(agstep_local_id)),
1024+
value: Box::new(Expr::Undefined),
1025+
is_error: false,
1026+
})));
1027+
} else {
1028+
// Sync generators re-drive the finally inline in this closure —
1029+
// no microtask suspend is needed (they have no `await`), and the
1030+
// finally's `yield`s return `{value, done: false}` directly.
1031+
return_resume_body.push(Stmt::While {
1032+
condition: Expr::Bool(true),
1033+
body: while_body_for_return,
1034+
});
1035+
}
10011036
} else {
10021037
return_resume_body.extend(return_fallback);
10031038
}

0 commit comments

Comments
 (0)