fix(gc): root the iterator drain's live values across .next() (#7475)

[proggeramlug]

Fixes #7475.

The filed hypothesis was wrong, and that matters

#7475 was filed on the reading that the auto-optimize path's feature-stripped
runtime archive (--no-default-features --features full,alloc-mimalloc) was
missing something. It isn't. Nothing in the feature set gates GC behaviour
here, and no missing feature needs adding.

The real cause is a #7341-family rooting bug that is present in the ordinary
build too. js_iterator_to_array — the drain behind [...iterable] /
Array.from — held five live GC values in bare Rust locals across a
.next() call that allocates the { value, done } result: the iterator
object, the accumulator array, the next closure, and the two property keys.
One level down, make_iter_result held the caller's element value and its own
freshly allocated result object across four more allocations before storing
them. And dispatch_array_iterator_method re-used a backing-array pointer read
before its cursor store, which can allocate.

Any of those allocations can trigger the copying minor. It moves the values and
rewrites only the slots it can see; a bare Rust local is not one. A moved
iterator leaves its pre-move copy in retired from-space, so the next dispatch
reads THAT copy's field 0 — the backing array — and
dispatch_array_iterator_method calls js_array_length on a from-space
address.

Trigger, not cause: this was latent for every user

The distinction decides who was exposed, so it was measured rather than argued.

Both links have the defect. PERRY_GC_PROTECT_FROMSPACE=1 faults on the
PERRY_NO_AUTO_OPTIMIZE=1 binary too — the one that prints the correct
checksum — at the same retiring minor (#0):

[gc-fromspace-protect] FAULT: signal 10 at 0x286e5610298
  last-known object: user_ptr=0x286e56102a0 obj_type=1 size=40
2  js_array_length + 380
3  perry_runtime::array::iter_object::dispatch_array_iterator_method + 1928
4  perry_runtime::object::native_call_method::handle_methods::dispatch_handle + 4264
5  js_native_call_method + 17864
6  js_iterator_to_array + 472
7  main + 1368

That binary "passed" only because the stale read landed on bytes that had not
yet been recycled. Every user of array spread / Array.from was exposed;
the auto-optimize link merely allocates in an order that makes the stale read
observable.

Isolated by rebuilding the runtime archive one axis at a time and linking each
through the plain no-auto path, so the link line was identical in all four rows:

runtime archive	object_deep_clone
default features, default RUSTFLAGS	checksum 1249975000
default features, auto-opt RUSTFLAGS (-C panic=abort -C force-unwind-tables=yes)	checksum 1249975000
auto-opt features (--no-default-features --features full,alloc-mimalloc), default RUSTFLAGS	TypeError: next is not a function
auto-opt features + auto-opt RUSTFLAGS (i.e. the real thing)	TypeError: next is not a function

RUSTFLAGS are irrelevant; the feature set is an exposure change. Runtime knobs
agree it is the copying minor and not policy evacuation: PERRY_GEN_GC=0,
PERRY_GC_SCAVENGE=0 and PERRY_WRITE_BARRIERS=0 each make the failing binary
pass, while PERRY_GEN_GC_EVACUATE=0 does not.

Fix

Root them in a RuntimeHandleScope and re-read every address at its point of
use (crates/perry-runtime/src/array/iterator.rs,
crates/perry-runtime/src/array/iter_object.rs). The per-iteration result
object gets one reusable scratch slot rather than a fresh handle per turn — the
loop runs up to 100k times. make_iter_result / make_sqlite_iter_result
collapse into one rooted build_iter_result.

Raw-handle debt: green, and no new bare reads. python3 scripts/raw_handle_debt.py reports 999 (baseline 999) — unchanged, with the
zero headroom the gate has. crates/perry-runtime/src/array/iterator.rs and
iter_object.rs have no ceiling entry, so a single bare
get_raw_{mut,const}_ptr in either would turn it red. The diff adds none: every
handle is NaN-boxed and read back with get_nanbox_f64 / get_nanbox_u64
(which the ratchet does not count and which cannot go stale in the way a raw
copy does), and the one read that follows an allocating call uses the
combinator form, step_h.across_nanbox(|| …). The only get_raw_const_ptr
string the diff introduces is inside a doc comment explaining why it is avoided.

(across_const was the first attempt there and panics at runtime on a NaN-boxed
handle — runtime handle kind mismatch: expected raw pointer. Loudly, which is
how it was caught; across_nanbox is the right combinator for a Nanbox slot.)

Regression coverage

test-files/test_gap_gc_iterator_drain_rooting.ts, registered in
test-parity/gc_repsel_corpus.txt so gc-moving-witnesses runs it and refuses
a cell where nothing moved. It mirrors the kernel deliberately: deepClone's
size is what keeps it out of line, and an out-of-line spread behind two property
hops is the lowering that calls js_iterator_to_array directly.

	before	after
default-feature link	TypeError: next is not a function	byte-exact with the oracle
auto-optimize link	TypeError: next is not a function	byte-exact with the oracle
PERRY_GC_PROTECT_FROMSPACE=1 DEPTH=200	FAULT at js_array_length inside dispatch_array_iterator_method, both links	that fault gone; a different one remains (#7498)

The instrument is not clean after this fix, and claiming otherwise would be
the overclaim this repo keeps paying for. [...o.meta.tags] reaches the drain
through array_from_spread_value, whose [Symbol.iterator] prototype walk
has its own stale from-space deref — a 56-byte GC_TYPE_STRING at minor #3.
That is #7498, unfixed here. It does not corrupt this file's result, so the
witness gates the drain and nothing else; test-parity/gc_repsel_corpus.txt
records that a protected run should go silent when #7498 lands, and that a
remaining fault would mean a third site.

scripts/auto_opt_app_patterns.sh + .github/workflows/auto-opt-app-patterns.yml
close the blind spot that let this ship. Every other gate in the repo sets
PERRY_NO_AUTO_OPTIMIZE=1 for a deterministic link — gc-ratchet says so
inline, and so do a dozen crates/perry/tests cases — so the default path, the
one users actually get, was tested by nothing.

The gate asserts its subject was live: it reads the linker command line out
of perry -v and requires it to name a perry-auto-*/…/libperry_runtime.a that
exists on disk. The auto-optimizer falls back to the prebuilt archives by design
whenever its cargo rebuild fails, and such a run would pass every output
comparison while exercising the exact configuration this job does not care
about. --self-test proves that matcher can still fail — and it already
caught a real over-match: the first version grepped the whole log and accepted
a canned fallback run that printed auto-optimize: built … and then linked the
prebuilt archive. It now reads only the [link] invoking: line, and CI runs the
self-test before the kernels.

Measured on this branch: 11 of 12 app-pattern kernels PASS through the
auto-optimize link, each linking a freshly built perry-auto-* archive and
matching the node oracle byte for byte. promise_all_chains is the one
documented skip, and a skip entry naming a kernel that no longer exists fails
the script, so the line cannot outlive its fix.

Per CLAUDE.md's corollary the workflow is deliberately not added to branch
protection's required contexts in this PR — a new gate has never been green.
Promote it after its first green run on main.

What is left, and where it lives

#7475 closes with this merge. Its two residual defects are not a vague
remainder; each now has its own issue, with a repro and a first thing to check:

• app-patterns: promise_all_chains rejects with a resolution value at scale (Uncaught (in promise) 0) #7497 — promise_all_chains rejects with a resolution value at scale.
  Fails under both links, differently (Uncaught (in promise) 0 with
  PERRY_NO_AUTO_OPTIMIZE=1; Uncaught (in promise) TypeError: value is not a function without), and is unchanged by this PR in both, which is the
  evidence that it is a promise-rejection defect and not the rooting family.
  Correct at N_BATCHES=3, BATCH_SIZE=5. This is the remaining blocker on the
  public benchmark artifact, and the one skip in the new gate.
• gc: array_from_spread_value's Symbol.iterator lookup derefs a from-space object (inlined [...obj.arr]) #7498 — array_from_spread_value's Symbol.iterator lookup derefs a
  from-space object. [...obj.arr] has two lowerings; when the enclosing
  function is small enough to inline, the spread routes through
  array_from_spread_value rather than js_iterator_to_array, and
  PERRY_GC_PROTECT_FROMSPACE=1 faults inside the prototype walk
  (array_prototype_property_value → default_object_prototype_property_value
  → js_object_get_field_by_name). Reproduces on this branch with the shipped
  witness, so no shrinking is needed: the two [...obj.arr] lowerings converge —
  array_from_spread_value ends by calling js_iterator_to_array — and the
  spread reaches this prototype walk first either way. Both links, so every user
  is exposed. Table-vs-register is open: the two traces differ in both the stale
  object (72-byte GC_TYPE_OBJECT vs 56-byte GC_TYPE_STRING) and the retiring
  minor (#0 vs #3), so CLAUDE.md's "deterministic at minor #0 ⇒ a table"
  shortcut does not apply.

Review

All four CodeRabbit findings accepted and applied; each has an inline reply. Two
were real rooting bugs in the fix itself and are worth naming here:

• js_iterator_to_array rooted iter_f64 after js_array_alloc(8) — an
  allocation, so a copying minor could move the iterator while it existed only
  in the raw argument, and the handle would then root a pre-move address. The
  same mistake the PR is about, one line earlier than where I fixed it.
• dispatch_array_iterator_method re-derived arr_ptr from field 0 after its
  cursor store but used the raw iter_obj parameter to do so, which the
  same store could have invalidated. Half a fix. It now roots the receiver at
  entry and reads the current address through a shadowing iter_obj() closure,
  so the pre-collection address is not nameable after that line.

The other two: the gate now refuses when the node oracle disagrees with
.node-version (the oracle version is a correctness input, and
gc_repsel_matrix.sh refuses on the same grounds), and the workflow's relevance
filter also matches .github/actions/setup-llvm22/.

Validation

• object_deep_clone through the real auto-optimize path (no
  PERRY_NO_AUTO_OPTIMIZE, freshly built perry-auto-* archive linked):
  checksum: 1249975000, matching node and bun.
• scripts/auto_opt_app_patterns.sh over all twelve kernels: 11 PASS, 1
  documented SKIP; --self-test green.
• cargo fmt --all -- --check, scripts/check_file_size.sh,
  scripts/check_test_registration.py, scripts/raw_handle_debt.py (999,
  unchanged) and its --self-test all clean.

https://claude.ai/code/session_019EHcmXKArA7m42SihYCcgH

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 38 seconds

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: 24fc0982-f5b4-4c58-80f5-0730785ad3df

📥 Commits

Reviewing files that changed from the base of the PR and between eb8a9ac and 37384d3.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (9)

• .github/workflows/auto-opt-app-patterns.yml
• CLAUDE.md
• Cargo.toml
• changelog.d/7495-auto-opt-archive.md
• crates/perry-runtime/src/array/iter_object.rs
• crates/perry-runtime/src/array/iterator.rs
• scripts/auto_opt_app_patterns.sh
• test-files/test_gap_gc_iterator_drain_rooting.ts
• test-parity/gc_repsel_corpus.txt

📝 Walkthrough

Walkthrough

Changes

The runtime roots iterator values across moving-GC allocations and refreshes relocated pointers. A regression test covers iterator draining during deep cloning. A Bash gate and GitHub Actions workflow validate auto-optimized app-pattern kernels against Node.

Iterator GC safety and validation

Layer / File(s)	Summary
Root and refresh iterator values crates/perry-runtime/src/array/iter_object.rs, crates/perry-runtime/src/array/iterator.rs, changelog.d/7495-auto-opt-archive.md	Iterator-result construction and iterator draining use RuntimeHandleScope roots, reusable scratch storage, and refreshed pointers after allocations.
Add iterator GC regression coverage test-files/test_gap_gc_iterator_drain_rooting.ts, test-parity/gc_repsel_corpus.txt	A 50,000-item deep-clone test exercises iterator draining and records expected parity behavior across GC configurations.
Implement auto-optimization kernel gate scripts/auto_opt_app_patterns.sh, changelog.d/7495-auto-opt-archive.md	The script discovers kernels, validates skip entries, compiles with AUTO-OPTIMIZE, verifies the linked runtime archive, and compares output with Node.
Wire the CI workflow .github/workflows/auto-opt-app-patterns.yml	The workflow filters relevant pull requests, prepares toolchains and runtime archives, and runs the auto-optimization gate on pull requests, main pushes, and manual dispatches.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant GitHub Actions
  participant auto_opt_app_patterns.sh
  participant Perry
  participant Node
  GitHub Actions->>auto_opt_app_patterns.sh: Run selected app-pattern kernels
  auto_opt_app_patterns.sh->>Perry: Compile with AUTO-OPTIMIZE
  Perry-->>auto_opt_app_patterns.sh: Compiled output and linked runtime archive
  auto_opt_app_patterns.sh->>Perry: Execute compiled kernel
  auto_opt_app_patterns.sh->>Node: Execute oracle kernel
  Perry-->>auto_opt_app_patterns.sh: Kernel output
  Node-->>auto_opt_app_patterns.sh: Oracle output
  auto_opt_app_patterns.sh-->>GitHub Actions: Pass or failure status

Loading

Possibly related PRs

• PerryTS/perry#6990: Addresses GC relocation and rooting during allocation-heavy runtime operations.
• PerryTS/perry#7138: Addresses moving-GC correctness in Perry’s array and iterator runtime.
• PerryTS/perry#7345: Adds related RuntimeHandleScope rooting in iterator code.

Suggested labels: parity

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly identifies the main change: fixing GC rooting for iterator draining across allocating .next() calls.
Description check	✅ Passed	The description thoroughly explains the cause, fix, regression coverage, validation, scope, and related issues, despite not following every template heading.

✨ Finishing Touches 💡 2

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/7475-auto-opt-archive

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 4

🤖 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 @.github/workflows/auto-opt-app-patterns.yml:
- Around line 91-103: Update the relevant-path grep expression in the workflow’s
changed-file gate to match `.github/actions/setup-llvm22/` and any files beneath
it. Preserve the existing paths and run=true behavior so changes to the LLVM
setup action trigger the build and kernel steps.

In `@crates/perry-runtime/src/array/iter_object.rs`:
- Around line 658-665: Root iter_obj at the start of the containing iterator
function using a RuntimeHandleScope, rather than retaining the raw parameter
across allocations. After every allocating operation, especially
js_object_set_field, reload iter_obj from the handle before reading field 0 or
otherwise dereferencing it, then derive arr_ptr from the reloaded object.

In `@crates/perry-runtime/src/array/iterator.rs`:
- Around line 1149-1160: Move creation of the rooted iterator handle immediately
after RuntimeHandleScope::new(), before js_array_alloc(8). Derive iter_ptr from
iter_h and use iter_h for all subsequent iterator accesses, including the null
check, so the iterator value is rooted before allocation and reloaded after
potential GC.

In `@scripts/auto_opt_app_patterns.sh`:
- Around line 47-48: Update the startup flow in auto_opt_app_patterns.sh after
resolving NODE_BIN to read the pinned version from .node-version and compare it
with the selected Node executable. Reject mismatches with a clear error and exit
before compilation or oracle/kernel execution, while preserving the existing
NODE_BIN override behavior.

🪄 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: dbb4c949-2e07-437b-a5bd-5a3c094ab4fb

📥 Commits

Reviewing files that changed from the base of the PR and between a1fcdeb and 147aeff.

📒 Files selected for processing (7)

• .github/workflows/auto-opt-app-patterns.yml
• changelog.d/7495-auto-opt-archive.md
• crates/perry-runtime/src/array/iter_object.rs
• crates/perry-runtime/src/array/iterator.rs
• scripts/auto_opt_app_patterns.sh
• test-files/test_gap_gc_iterator_drain_rooting.ts
• test-parity/gc_repsel_corpus.txt

[proggeramlug]

Review round applied in 5649e8e — all four CodeRabbit findings accepted, each with an inline reply. Two were real rooting bugs in the fix itself: js_iterator_to_array rooted the iterator after js_array_alloc(8), and dispatch_array_iterator_method re-derived arr_ptr from field 0 using the raw iter_obj parameter that the same cursor store could have invalidated.

Re-validated on the rebuilt archives:

auto-optimize app-pattern gate: 11 kernel(s)
PASS  batch / buffer_transcode / date_format_parse / json_parse_1mb /
      json_stringify_1mb / map_1m / object_deep_clone / regex_replace /
      string_concat_csv / string_split_map_join / string_template_interp
SKIP  promise_all_chains — promise rejects with a resolution value at scale, #7497
OK: every selected kernel linked a freshly built perry-auto runtime archive
    and matched the node oracle byte for byte.

--self-test green, scripts/raw_handle_debt.py still 999, cargo fmt --all -- --check clean.

One correction to an earlier claim in this PR body. I had written that a PERRY_GC_PROTECT_FROMSPACE=1 run of the witness is clean after the fix. It is not, and I have fixed the row rather than leave it. The drain's fault (js_array_length inside dispatch_array_iterator_method) is gone; a different from-space deref remains in array_from_spread_value's [Symbol.iterator] prototype walk — a 56-byte GC_TYPE_STRING at minor #3. That is #7498, unfixed here, and it does not corrupt the witness's output. test-parity/gc_repsel_corpus.txt now records both facts, plus the follow-up test: a protected run of test_gap_gc_iterator_drain_rooting should go silent when #7498 lands, and a remaining fault would mean a third site.

gc-ratchet is red on this PR, and is pre-existing — the same harness unit test (test_pinned_artifact_retention_is_deterministic, 12_large_live_set.heap_used_bytes was not deterministic when pinned) fails on main at this PR's merge base a1fcdeb. It reads a checked-in artifact and is untouched by this diff.

[proggeramlug]

Second pre-existing red, for the record: gc-root-dominance fails on its --audit-immovable-sources premise probe —

[FAIL] old-page defrag is off by default: gc/oldgen.rs no longer mentions
       PERRY_GC_OLD_DEFRAG; old-page defrag may now be unconditional

crates/perry-runtime/src/gc/oldgen.rs is not in this diff; it was last touched by #7449 (the recycled-block pool), which is what dropped the PERRY_GC_OLD_DEFRAG mention the probe keys on. The same job is red on main at this PR's merge base a1fcdeb, alongside gc-ratchet. Both are unrelated to this change and neither is silent — they are doing their job, just about someone else's commit.

The new auto-opt-app-patterns job triggers correctly on every push to the branch (earlier runs were cancelled by its own cancel-in-progress on pull-request events, which is the intended scoping); the run for 5649e8e is in progress.