Skip to content

fix(codegen): root child_process command/file across their optional operands - #7465

Merged
proggeramlug merged 2 commits into
mainfrom
fix/layer1-child-proc-rooting
Aug 5, 2026
Merged

fix(codegen): root child_process command/file across their optional operands#7465
proggeramlug merged 2 commits into
mainfrom
fix/layer1-child-proc-rooting

Conversation

@proggeramlug

Copy link
Copy Markdown
Contributor

Closes the last real sites of this shape in perry-codegen.

The bug

exec / execFile / execFileSync each unboxed the command or file to a raw StringHeader pointer and then lowered options/args/callback — arbitrary user code — before the runtime call used it. The intermediate operands have the same problem: args_v crosses the options lowering, opts_v crosses the callback lowering.

Each operand is now rooted as it is produced and reloaded after the last lowering.

One truncate per arm, verified not assumed

The file reads 8 pushes / 3 truncates, which looks like a leak. It isn't: js_gc_temp_root_truncate is s.truncate(base), so truncating to the outermost slot drops every slot pushed after it. I checked the runtime rather than trusting the name — having already shipped one release-path leak today (#7463), a plausible-looking count was not something to accept.

Validate ordering is preserved: emit_cp_validate_command still runs on the freshly lowered value, before the root is taken.

The crate is now clean of this shape

An arm-aware rescan reports one remaining site, in native_ui_widgets_branch.rs, and it is not a bug: perry/ui widgets are 1-based registry handles below the handle band, which the collector never moves — the same reason the inline guards elsewhere test > HANDLE_BAND_TOP before dereferencing.

The earlier scan also flagged i32_fast_path.rs. That was an artifact of the scanner treating a free function's body as a match arm and running past its end into the next function.

Verification

  • 5/5 child_process gap tests byte-identical to the pinned Node oracle
  • an execFileSync repro with an allocating argument matches node (len 32) and is unchanged under PERRY_GC_HEAP_LIMIT=8 PERRY_GC_FORCE_EVACUATE=1
  • cargo fmt --check clean

Ralph Küpper added 2 commits August 5, 2026 18:15
…perands

Closes the last real sites of this shape in perry-codegen.

exec/execFile/execFileSync each unboxed the command or file to a raw
StringHeader pointer and then lowered options/args/callback -- arbitrary
user code -- before the runtime call used it. The intermediate operands
have the same problem: args_v crosses the options lowering, opts_v
crosses the callback lowering.

Each operand is now rooted as it is produced and reloaded after the last
lowering. One truncate per arm releases them all: js_gc_temp_root_truncate
is s.truncate(base), so truncating to the outermost slot drops every slot
pushed after it -- verified in the runtime rather than assumed, since
"8 pushes / 3 truncates" reads like a leak otherwise.

Validate ordering is preserved: emit_cp_validate_command still runs on the
freshly lowered value, before the root is taken.

An arm-aware rescan of the whole crate now reports ONE remaining site,
in native_ui_widgets_branch.rs, and it is not a bug: perry/ui widgets are
1-based registry handles below the handle band, which the collector never
moves. The earlier scan also flagged i32_fast_path.rs, which was an
artifact of the scan treating a free function's body as a match arm.

5/5 child_process gap tests byte-identical to node; an execFileSync repro
with an allocating argument matches node and is clean under
PERRY_GC_HEAP_LIMIT=8 PERRY_GC_FORCE_EVACUATE=1.
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@proggeramlug, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 1 minute

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 81e2b784-d36d-4e7e-9508-2bb03cdd9f50

📥 Commits

Reviewing files that changed from the base of the PR and between 5b92c72 and 1fbff50.

📒 Files selected for processing (2)
  • changelog.d/7465-child-proc-rooting.md
  • crates/perry-codegen/src/expr/child_proc.rs

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@proggeramlug
proggeramlug merged commit bf75a63 into main Aug 5, 2026
1 check was pending
@proggeramlug
proggeramlug deleted the fix/layer1-child-proc-rooting branch August 5, 2026 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant