perf(transform): release a completed async activation's boxed locals (#7933) - #7939
Merged
Conversation
added 5 commits
August 12, 2026 10:46
The async-to-generator transform boxes every body local of an async function into a never-freed `BOX_REGISTRY` cell, and nothing ever cleared one — so every local of every activation the program had ever run stayed a live GC root. Clear (never free) the cells no closure can observe at the state machine's two terminal states. Refs #7933 Claude-Session: https://claude.ai/code/session_012B8z92S82sCfqCrVqrFgS2
📝 WalkthroughWalkthroughThe async-to-generator transform now analyzes closure-visible locals, clears safe boxed locals at terminal completion and rejection, retains registry addresses and control cells, and validates closure escapes, terminal paths, and emitted stores. ChangesAsync activation box release
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant AsyncFunction
participant AsyncLowering
participant AsyncStepDriver
participant BoxRelease
participant Promise
AsyncFunction->>AsyncLowering: lower generator body
AsyncLowering->>BoxRelease: analyze closure-visible locals
BoxRelease-->>AsyncLowering: release_ids
AsyncLowering->>AsyncStepDriver: pass release_ids
AsyncStepDriver->>BoxRelease: build release statements at terminal state
BoxRelease-->>AsyncStepDriver: clear boxed locals with undefined
AsyncStepDriver->>Promise: resolve or reject
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
added 2 commits
August 12, 2026 11:32
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@changelog.d/7939-async-box-release.md`:
- Around line 18-22: Rewrite the sentence beginning with “Terminal states of the
`was_plain_async` step driver” in the changelog so it has a complete main clause
while preserving the explanation that exactly two terminal states exist: the
`IterResultGetDone` resolve arm and the catch arm’s `isError` branch.
In `@crates/perry-transform/src/generator/box_release.rs`:
- Around line 177-196: The closure branch in scan_expr must continue walking
closure parameter defaults after scanning the body. Remove the early return
after scan_stmts(body, scan), allowing the subsequent walk_expr_children call to
process defaults while preserving the existing capture and body scanning.
🪄 Autofix
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: 4b18fa6c-5f98-4b85-aafa-1da0719c2047
📒 Files selected for processing (6)
changelog.d/7939-async-box-release.mdcrates/perry-transform/src/generator/box_release.rscrates/perry-transform/src/generator/lower.rscrates/perry-transform/src/generator/lower/async_step.rscrates/perry-transform/src/generator/mod.rstest-files/test_issue_7933_async_box_release_escapes.ts
Comment on lines
+18
to
+22
| Terminal states of the `was_plain_async` step driver, and there are exactly | ||
| two: the `IterResultGetDone` resolve arm (the body ran to a `return`, which | ||
| `prepend_done_before_returns` pairs with `__gen_done = true`) and the catch | ||
| arm's `isError` branch (an exception escaped with no user `catch` to take it, | ||
| so the activation rejects). Everything else either suspends |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the sentence fragment.
Line 18 starts "Terminal states of the was_plain_async step driver, and there are exactly two:" — the clause has no main verb. This text appears in the assembled release notes.
✏️ Proposed fix
- Terminal states of the `was_plain_async` step driver, and there are exactly
- two: the `IterResultGetDone` resolve arm (the body ran to a `return`, which
+ The `was_plain_async` step driver has exactly two terminal states: the
+ `IterResultGetDone` resolve arm (the body ran to a `return`, which
`prepend_done_before_returns` pairs with `__gen_done = true`) and the catch
arm's `isError` branch (an exception escaped with no user `catch` to take it,
so the activation rejects).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Terminal states of the `was_plain_async` step driver, and there are exactly | |
| two: the `IterResultGetDone` resolve arm (the body ran to a `return`, which | |
| `prepend_done_before_returns` pairs with `__gen_done = true`) and the catch | |
| arm's `isError` branch (an exception escaped with no user `catch` to take it, | |
| so the activation rejects). Everything else either suspends | |
| The `was_plain_async` step driver has exactly two terminal states: the | |
| `IterResultGetDone` resolve arm (the body ran to a `return`, which | |
| `prepend_done_before_returns` pairs with `__gen_done = true`) and the catch | |
| arm's `isError` branch (an exception escaped with no user `catch` to take it, | |
| so the activation rejects). Everything else either suspends |
🤖 Prompt for 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.
In `@changelog.d/7939-async-box-release.md` around lines 18 - 22, Rewrite the
sentence beginning with “Terminal states of the `was_plain_async` step driver”
in the changelog so it has a complete main clause while preserving the
explanation that exactly two terminal states exist: the `IterResultGetDone`
resolve arm and the catch arm’s `isError` branch.
Comment on lines
+177
to
+196
| Expr::Closure { | ||
| body, | ||
| captures, | ||
| mutable_captures, | ||
| .. | ||
| } => { | ||
| scan.out.extend(captures.iter().copied()); | ||
| scan.out.extend(mutable_captures.iter().copied()); | ||
| let mut refs: Vec<LocalId> = Vec::new(); | ||
| let mut visited: HashSet<usize> = HashSet::new(); | ||
| perry_hir::analysis::collect_local_refs_expr(expr, &mut refs, &mut visited); | ||
| scan.out.extend(refs); | ||
| // Keep descending: nested closures contribute their own explicit | ||
| // capture lists, and a `with` anywhere inside must still poison. | ||
| scan_stmts(body, scan); | ||
| return; | ||
| } | ||
| _ => {} | ||
| } | ||
| perry_hir::walker::walk_expr_children(expr, &mut |child| scan_expr(child, scan)); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate the walker and inspect the Closure arm of walk_expr_children.
fd -t f 'walker.rs' crates/perry-hir/src | while IFS= read -r f; do
echo "== $f"
rg -n -C 12 'Expr::Closure' "$f"
done
# Confirm whether collect_local_refs_expr descends into closure param defaults.
rg -n -C 20 'fn collect_local_refs_expr' crates/perry-hir/srcRepository: PerryTS/perry
Length of output: 3310
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '== walker definitions =='
rg -n -C 25 'pub fn walk_expr_children|fn walk_expr_children|Expr::Closure' crates/perry-hir/src
printf '%s\n' '== collect_local_refs_expr =='
sed -n '35,150p' crates/perry-hir/src/analysis.rs
printf '%s\n' '== box_release scan =='
sed -n '120,220p' crates/perry-transform/src/generator/box_release.rs
printf '%s\n' '== related closure walkers =='
rg -n -C 12 'walk_expr_children_mut|scan_expr|scan_stmts' crates/perry-transform/src/generator/per_iteration.rs crates/perry-transform/src/generator/box_release.rsRepository: PerryTS/perry
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '== walker files =='
git ls-files | rg '(^|/)walker(\.rs|/)|walk.*\.rs$'
printf '%s\n' '== walker function locations =='
rg -n '^(pub )?fn walk_expr_children|walk_expr_children_mut' crates/perry-hir/src --glob '*.rs' | head -80
printf '%s\n' '== closure walker implementation =='
file=$(rg -l '^(pub )?fn walk_expr_children' crates/perry-hir/src --glob '*.rs' | head -1)
echo "$file"
line=$(rg -n '^(pub )?fn walk_expr_children' "$file" | head -1 | cut -d: -f1)
start=$((line-10)); end=$((line+180))
sed -n "${start},${end}p" "$file"
printf '%s\n' '== collect_local_refs_expr complete relevant section =='
rg -n 'collect_local_refs_expr|Expr::Closure' crates/perry-hir/src/analysis.rs
printf '%s\n' '== box_release relevant section =='
rg -n -C 10 'Expr::Closure|fn scan_expr|fn scan_stmts' crates/perry-transform/src/generator/box_release.rsRepository: PerryTS/perry
Length of output: 5175
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '== mutable walker =='
cat -n crates/perry-hir/src/walker/expr_mut.rs
printf '%s\n' '== read-only walker =='
cat -n crates/perry-hir/src/walker/expr_ref.rs
printf '%s\n' '== box_release scan functions =='
rg -n -C 16 'Expr::Closure|fn scan_expr|fn scan_stmts' crates/perry-transform/src/generator/box_release.rs
printf '%s\n' '== local-reference collector =='
sed -n '35,125p' crates/perry-hir/src/analysis.rsRepository: PerryTS/perry
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '== closure arms in walkers =='
rg -n -C 8 'Expr::Closure' crates/perry-hir/src/walker/expr_mut.rs crates/perry-hir/src/walker/expr_ref.rs
printf '%s\n' '== box_release closure scan =='
rg -n -C 20 'Expr::Closure|fn scan_expr|fn scan_stmts' crates/perry-transform/src/generator/box_release.rs
printf '%s\n' '== local-reference collector closure handling =='
rg -n -C 18 'Expr::Closure|pub fn collect_local_refs_expr' crates/perry-hir/src/analysis.rsRepository: PerryTS/perry
Length of output: 20111
Do not return before walking closure parameter defaults.
walk_expr_children visits closure parameter defaults but not the body. The early return therefore misses WithGet/WithSet poisoning and explicit captures from nested closures in parameter defaults. Remove return and fall through after scan_stmts(body, scan).
🤖 Prompt for 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.
In `@crates/perry-transform/src/generator/box_release.rs` around lines 177 - 196,
The closure branch in scan_expr must continue walking closure parameter defaults
after scanning the body. Remove the early return after scan_stmts(body, scan),
allowing the subsequent walk_expr_children call to process defaults while
preserving the existing capture and body scanning.
Source: Learnings
This was referenced Aug 12, 2026
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.
Closes #7933.
What was wrong
The async-to-generator transform boxes every body local of every
asyncfunction into a
js_box_alloc_bitscell (Stmt::PreallocateBoxes, one cellper local per invocation) so the synthesized state-machine closures can share
them across suspends. The runtime's
BOX_REGISTRYis monotonic by design —cells are never freed, because perry#4898's pointer rejection and #7906's
positive pointer cache both rest on "a registered address can never become
unregistered" — and
scan_box_roots_mutmarks the JSValue inside everyregistered cell on every collection.
Nothing ever cleared one. So every local of every activation the program had
ever run stayed a live GC root for the life of the process. On
asyncpipethat is the whole
Req/Ok/note graph of every batch the program has alreadydiscarded.
What this does
Clear (never free) an activation's cells at the state machine's terminal
states. A cleared cell stays registered and readable; a stale reader sees
undefined, which is already the defined value of an uninitialised boxed local(perry#4926).
BOX_REGISTRYmonotonicity, perry#4898 and #7906 are untouched.The exit paths, and how each is covered
build_async_step_driver_directhas exactly two terminalreturnsites:return(incl. implicit)IterResultGetDoneresolve arm — andprepend_done_before_returnsalready pairs every return with__gen_done = true, so the two predicates agreethrowwith no usercatchisErrorbranch (Promise.reject)throwfrom a user catch routethrowcaught by a usertry/catchin the async fnawaitsuspensionAsyncStepChain__step_self(e, true)returnposition, so the caller does nothing after the callee reaches a terminalawaiton a promise that never settles)A resume that still arrives after a terminal is harmless: it writes
__gen_sentbefore reading it, then short-circuits on__gen_done— which isdeliberately not released, since an
undefinedthere would drop it into thedispatch loop with no matching state.
__gen_state,__gen_executingand thepending-completion record are held back for the same reason.
Async generators and sync generators are deliberately untouched. Their
{next, return, throw}object is user-visible, so "done" is not the end ofobservability (
gen.return()can still re-run a pendingfinallythat readsthe locals).
Why releasing is safe — and where it stops
Clearing a cell whose value is still reachable is a silent wrong answer, not a
crash, so a cell is released only when no closure can hold its address.
A box address is never a JS value —
LocalGet/LocalSeton a boxed local lowerto
js_box_get/js_box_set— so it leaves the activation solely through aclosure capture slot, which codegen fills for exactly the ids in
compute_auto_captures(closure) ∩ boxed_vars, andcompute_auto_capturesisexplicit captures ∪ collect_ref_ids_in_stmts(closure body).New
generator/box_release.rs::closure_visible_idscomputes a superset ofthat:
captures ∪ mutable_captures ∪ perry_hir::analysis::collect_local_refs_expr(closure)(descending into nestedclosures). An id it misses is an id codegen's own free-variable walk also
misses — so no capture slot for that id exists and releasing its cell is
unobservable. The argument does not need my walker to be complete, only no less
complete than the one codegen already trusts for the same decision.
Sloppy-mode
withis the single construct that breaks it:Expr::WithGet/Expr::WithSetcarry a fallbackLocalIdas a leaf field thatcollect_local_refs_exprdoes not report. Either one anywhere in the bodypoisons the analysis and nothing is released.
The scan runs twice and unions — over the original body and over the
post-linearization resume body + catch routes.
Effect
asyncpipeat 240 batches (the shipped corpus program is at 120, which#7926 shows sits on a trigger cliff; 240 and 480 are both used here). Same
compiler flags, same runtime dir per arm, output byte-identical to the
baseline's and exit 0 in every cell.
young_survival_permillecopied_objectsfreed_byteseden_live_bytes@ minor #1★ Read the adjacent counters:
copied_objectsis structurally zero on apromoting cycle, so both
copied_objectsandpromoted_objectsare quoted.Instructions retired is quoted as a load-independent cost proxy, not as a
speedup — the dev box ran at load 25–90 throughout.
The measured live young set (~6 600 objects) matches the unsound
PERRY_PROBE_BOX_NOMARKprobe in #7933 to within noise — but here the programexits 0 with the correct output, where the probe SIGABRTed.
Validation
both arms (
gc-handoff/boxclear/build_corpus.sh). Onlyasyncpipein thatcorpus contains an
asyncfunction, so the other 18 are a no-regressioncheck.
test-files/test_issue_7933_async_box_release_escapes.ts— eightshapes where a closure outlives the activation that declared its local
(returned closure, mutating closure, closure created before the
await,closure stored in an escaping object, closure in a nested
asyncarrow,closure created on a catch path, closures pushed from a loop, and a closure
carried on a rejected activation's thrown value). Byte-identical to node on
both arms.
many later collections) is byte-identical to node on both arms and runs a
real copying minor — 391 ‰ → 135 ‰ survival, so the release fires there too.
PERRY_GC_PROTECT_FROMSPACE=1 PERRY_GC_PROTECT_FROMSPACE_DEPTH=800on thescaled probe: exit 0, correct output, and the instrument was live —
[gc-fromspace-protect] mode=ProtectPages retired_set=#0 blocks=18 bytes_protected=18874368.PERRY_GC_VERIFY_EVACUATION=1onap_240and the scaled probe: exit 0,correct output.
test-files/*.tscontainingasync/await— 213 files,two compilers, byte-comparing stdout, stderr and exit code.
---- ran=213 diffs=8 basebuildfail=10 fixbuildfail=10 ----, and all 8flagged diffs are the OS thread id inside an identical panic message on
pre-existing network/server failures that abort on both arms
(
perry-ext-http/src/server/server.rs:911,perry-ext-axios/src/lib.rs:94,tokio/net/addr.rs:182). Normalising the thread id leaves 0 real diffs;stdout was already identical in all 8. The 10 build failures are the same 10
files on both arms (base-only
[], fix-only[]).cargo test -p perry-transform: 83 + 10 passed, 0 failed.cargo fmt --all -- --check,scripts/check_file_size.sh.New unit tests
generator/box_release.rscarries both directions, including two end-to-endtests that drive the real
transform_async_to_generator+transform_generatorspipeline:a_confined_body_local_is_released_at_the_terminal_states— two releasestores, one per terminal arm.
a_body_local_a_closure_can_see_is_never_released— the same local, but aclosure escapes with it: zero stores.
the_state_machine_control_locals_are_not_released.with-poison cases.Summary by CodeRabbit
Bug Fixes
Tests