fix(object): resolve the array forwarding chain before reading a header in Object.* (#7548)

[proggeramlug]

Fixes #7548.

What was wrong

js_array_grow reallocates an array's header+elements as one allocation and leaves a #233 forwarding stub at the old address. The stub's first 8 bytes are exactly where length and capacity live, so they read back as the two halves of the forwarding pointer.

The array branches of Object.* reinterpreted the caller's pointer with a bare obj as *ArrayHeader cast. Any JS binding still holding an array's pre-grow address therefore made (*arr).length return a heap address — 615,098,568 instead of 6 in the observed case (capacity read back as 1273, i.e. the two words together were a ~4.9 TB mimalloc address).

is_array_object cannot tell a stub apart: it keeps obj_type == GC_TYPE_ARRAY, and only the GC_FLAG_FORWARDED bit plus the clobbered payload distinguish it. So the bad pointer sailed through every guard.

Two loops are driven by that length and became bounded-but-unreachable walks — one to_string() plus an attrs side-table probe per index:

loop	reached from
mark_all_array_props	Object.freeze / Object.seal of any array that has ever outgrown its dense capacity. const t=[1,2]; t.push(3); Object.freeze(t) never returns.
array_set_length_from_descriptor	ArraySetLength's shrink walk, via the Set(receiver,"length",n) tail of an Array.prototype.splice that grows a Proxy receiver — the reported timeout.

The hang is not infinite. It is a bounded loop over ~6·10^8 iterations; the harness's 10 s budget simply cannot distinguish that from non-termination. Getting this right mattered: the fix for "slow" and the fix for "non-terminating" are different, and pattern-matching on "trap re-entering itself" would have been wrong — the mutator's element writes all completed, and it was the final length write that walked.

Narrowing

test_gap_6908_proxy_array_mutators.ts stops after sort-cmp, i.e. in the first splice block, not the dense/object-like section the issue guessed. Tracing proxy_set_str_key showed all five element writes completing and the trailing Set(proxy,"length",6) never returning. That reduced to a repro with no splice, no mutator, and no Proxy at all:

const t = [1, 2, 3, 4, 5];
Object.defineProperty(t, "5", { value: 5, writable: true, enumerable: true, configurable: true });
Object.defineProperty(t, "length", { value: 6 });   // walks 6·10^8 times

and then to the strictly worse Object.freeze case above, which needs no defineProperty either.

Fix

One array_header / array_header_mut helper that walks the forwarding chain (via clean_arr_ptr) before the cast, applied at all four header casts in array_object_ops.rs. It falls back to the raw cast when the chain does not resolve, so no caller loses a pointer it previously accepted.

Deliberately not changed: the obj as usize side-table keys. The array attrs table is keyed inconsistently across the runtime — getOwnPropertyDescriptor reads at the caller's (possibly pre-grow) address, while the element-write rejection path resolves through clean_arr_ptr first. I measured both ways: re-keying only these writers regressed getOwnPropertyDescriptor on a grown frozen array without gaining the write rejection. Unifying the readers is a separate change; see "Adjacent findings".

Sibling mutators on a Proxy receiver

The discriminator is not the mutator — it is whether the mutator writes an index at or beyond the receiver's dense capacity and then writes length.

affected (hung)	unaffected
push, unshift (always grow)	pop, shift (shrink)
splice when inserts > deletes	splice remove-only / equal-count
splice(len, 0, x) (pure append)	reverse, sort, fill, copyWithin (never grow)

reverse/fill/copyWithin — the other mutators #7424 touched — are clean in every form, including their .call forms.

Which commit

There is no bisectable regression commit — the test never passed. I built perry at d255ae604 (#7424, the PR that added test_gap_6908_proxy_array_mutators.ts) from an exported source tree in its own target dir, with the same -p set, and ran the test that commit ships: exit 137 after the identical five lines, last line sort-cmp: 2,4,10,33. It was broken on arrival, and no per-PR job could report it because parity is gated to tag pushes.

All four bare casts are present verbatim at that commit. They date to #4709 (2026-06-06, ArraySetLength) and #5025 (2026-06-11, freeze/seal on arrays) and were never touched since — so the Object.freeze-on-a-grown-array hang has been shipping for two months, entirely independent of any Proxy work.

Validation (local — CI is 130+ deep and may not run this)

• test_gap_6908_proxy_array_mutators.ts: byte-identical to node 26.5.1, exit 0 (was exit 124 after 5 of 25 lines).
• New test_gap_7548_grown_array_object_ops.ts: byte-identical, exit 0. The pristine arm hangs on it with zero output.
• New unit test stale_pre_grow_array_pointer_reads_the_real_length_in_object_ops is sabotage-tested: reverting array_header to the bare cast fails it in 0.00 s with left: 8913048 right: 17. It asserts non-vacuity too (the stub's length word must actually differ from the real length), and asserts the header read before the walks, so a regression fails fast instead of hanging the suite.
• cargo test -p perry-runtime --no-fail-fast: 1798 passed, 0 failed, 3 ignored.
• Targeted gap sweep, 167 tests matching Object.freeze|seal|defineProperty|getOwnPropertyDescriptor|preventExtensions|isFrozen|isSealed, Reflect.defineProperty|set|getOwnPropertyDescriptor, new Proxy, propertyIsEnumerable, .splice(, .push(, .unshift( — A/B'd against a pristine build in its own target dir. 159 PASS, 5 DIFF, 3 NODE_FAIL, 0 TIMEOUT — and every one of the 5 diffs is byte-identical between the two arms, so zero regressions. Two are already tracked (test_gap_2159_defineproperty_class_prototype in gap_snapshot.json + known_failures.json; test_gap_diagchannel_3082_3084_3085_3086 in known_failures.json); the other three are host-local flakes unrelated to this change (test_gap_http_overloads_3226plus panics identically in both arms at crates/perry-ext-http/src/server/server.rs:911, test_gap_zlib_3285_params and test_gap_stream_tee_tick_parity produce identical output in both arms).
• Gates: raw_handle_debt.py → 999 (baseline 999); check_file_size.sh OK; addr_class_inventory.py passed; cargo fmt --all -- --check clean.

Adjacent findings (not fixed here, reported for triage)

1. The array attrs side table is keyed inconsistently. getOwnPropertyDescriptor reads at the caller's address; the element-write rejection path reads at the clean_arr_ptr-resolved address. Consequence: after this PR, Object.freeze on a grown array terminates and reports correct descriptors, but the element write is still not rejected (t[0]=99 lands). That was previously invisible because the freeze hung. Strictly better than a hang, still not node-identical.
2. Array.prototype.push.call(proxy, …) and .unshift.call(proxy, …) silently no-op. Verified identical on the pristine arm, so pre-existing and unrelated.
3. getOwnPropertyDescriptor loses an accessor defined at an index past capacity (typeof d.get is undefined, node says function) — the mirror of finding 1, from the accessor branch's existing canonical re-keying. Also identical on the pristine arm.

Summary by CodeRabbit

• Bug Fixes

  • Fixed array operations after arrays grow and move in memory.
  • Improved reliability for freezing, sealing, length updates, property descriptors, enumeration, keys, and Proxy-based mutations.
  • Prevented incorrect array metadata from causing excessive processing or invalid results.

• Tests

  • Added regression coverage for grown arrays, reallocation, and related object operations.
  • Added behavior comparisons for common array operations, including mutations, descriptors, keys, and serialized contents.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 321df847-df37-4962-aab6-6eba26b807a2

📥 Commits

Reviewing files that changed from the base of the PR and between 9a77a26 and a184d0f.

⛔ Files ignored due to path filters (1)

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

📒 Files selected for processing (6)

• CLAUDE.md
• Cargo.toml
• changelog.d/7551-array-forwarding-stub-object-ops.md
• crates/perry-runtime/src/object/array_object_ops.rs
• crates/perry-runtime/src/object/tests.rs
• test-files/test_gap_7548_grown_array_object_ops.ts

---

📝 Walkthrough

Walkthrough

Array object operations now resolve stale array pointers through GC forwarding chains before reading array headers. Runtime and gap tests cover array property operations after growth, including freeze, seal, length updates, Proxy mutators, enumerability, keys, descriptors, and serialization.

Changes

Grown array object operations

Layer / File(s)	Summary
Array header resolution and property operations crates/perry-runtime/src/object/array_object_ops.rs	Added forwarding-chain helpers. Array marking, enumerability, length handling, and indexed property definition now use the current array header while preserving caller addresses for side-table keys.
Grown-array regression coverage crates/perry-runtime/src/object/tests.rs, test-files/test_gap_7548_grown_array_object_ops.ts, changelog.d/7551-array-forwarding-stub-object-ops.md	Added runtime and gap tests for stale pointers, array growth, Proxy mutators, descriptors, enumeration, and serialization. Documented the affected operations and coverage.
Release version update Cargo.toml, CLAUDE.md	Updated the package version and documented current version from 0.5.1310 to 0.5.1311.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ArrayObjectOps
  participant ForwardingChain
  participant CurrentArray
  ArrayObjectOps->>ForwardingChain: resolve stale array pointer
  ForwardingChain->>CurrentArray: follow forwarding chain
  CurrentArray-->>ArrayObjectOps: return current array header
  ArrayObjectOps->>CurrentArray: process array properties

Loading

Possibly related PRs

• PerryTS/perry#6907: Both cover Proxy array mutator handling and related array regression tests.
• PerryTS/perry#7424: Both test array operations involving Proxy receivers.
• PerryTS/perry#7540: Both address array forwarding-pointer resolution after growth.

Suggested reviewers: jdalton

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the primary fix: resolving array forwarding chains before reading headers in Object.* operations.
Description check	✅ Passed	The description covers the root cause, fix, linked issue, scope, validation, and regression evidence, despite not using every template heading.
Linked Issues check	✅ Passed	The PR fixes issue #7548, produces byte-identical Node output with exit 0, and establishes that the timeout came from a bounded but extremely large loop.
Out of Scope Changes check	✅ Passed	The code, changelog, and regression tests directly support the linked issue and stated forwarding-chain fix; no unrelated implementation changes are present.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/7548-proxy-mutator-hang

---

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: 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 `@crates/perry-runtime/src/object/array_object_ops.rs`:
- Around line 84-93: Unify the identity used by array attribute storage and
element-write lookup in the relevant array operation, using the
forwarding-resolved allocation from array_header or clean_arr_ptr consistently
instead of retaining the caller’s stale address. Update the descriptor-flag path
so grown frozen arrays are recognized by indexed writes, and add a regression
covering grow, freeze, then Reflect.set on index "0", asserting it returns false
and leaves the element unchanged.
- Around line 179-181: Update the array length-setting flow around
array_header_mut and js_number_coerce: root the receiver and descriptor before
coercion, then re-resolve the receiver after coercion and call array_header_mut
immediately before reading old_len or mutating the array. Do not retain raw
pointer locals across user-code-invoking operations, and add a regression
covering a length descriptor whose valueOf grows the target array.

🪄 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: b34cc455-5fd0-47c8-b2b7-326332d9ba43

📥 Commits

Reviewing files that changed from the base of the PR and between 9a77a26 and 3cf0bfe.

📒 Files selected for processing (4)

• changelog.d/7551-array-forwarding-stub-object-ops.md
• crates/perry-runtime/src/object/array_object_ops.rs
• crates/perry-runtime/src/object/tests.rs
• test-files/test_gap_7548_grown_array_object_ops.ts

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.