GC/array: #10077's dense-queue front offset is nonzero for an unshifted array, hanging test_gap_gc_http2_pending_event_callback_rooting (two independent readers) #10137

Activity

1. proggeramlug commented on Sep 12, 2026

   

   proggeramlug

   on Sep 12, 2026

   ContributorAuthor

   More actions

   Correction — my suspected mechanism is wrong; disregard it.

   I tested it rather than leaving it as a plausible-sounding theory. Guarding the new gc_element_slot_range comparison behind a tracked-header check (a array_physical_capacity_checked returning None for an untracked cell, so an unvalidated header read can no longer trip the capacity comparison and suppress the whole slot range) compiles clean and does not fix the hang — still CRASH/TIMEOUT, 2 of 2 runs. I have reverted that change and am not proposing it.

   So the "unguarded header read in array_physical_capacity makes the collector scan no element slots" story is not the cause of this bug. It may still be worth a look on its own merits — the read is unguarded where the branch directly below it validates via try_read_tracked_gc_header — but it is not what breaks this test, and I do not want that sitting on the issue as a lead for someone to spend time on.

   What remains established, all with full run_parity_tests.sh builds:

   • pristine main before the trains (435d6396c) — PASS
   • + train161 (c58c1fae5) — PASS
   • + train162 (dc0d876fe) — PASS
   • + all of #10075 (a816fd2e7) — PASS
   • + all of #10077 (9b518346a) — CRASH
   • current main (50e08e91dd) — CRASH

   #10077 introduces it; #10075 does not; the maintainer commit that landed above them does not. The failure is a hang, not a signal death: 0 lines of output, killed at the harness's 10 s timeout, Node exits 0.

   One more thing I checked and can rule out: the client_request_surface.rs hunk in fix(ffi): read shifted arrays through the existing accessor looks like it moves away from the offset-aware accessor to a hardcoded +8, but the pre-#10077 code there was array.add(1) as *const f64 — pointer arithmetic on *const ArrayHeader, i.e. the same address. That hunk is behaviour-neutral.
2. proggeramlug commented on Sep 12, 2026

   

   proggeramlug

   on Sep 12, 2026

   ContributorAuthor

   More actions

   Root cause found

   GcHeader.size is the PADDED allocation size, and #10077 derives the queue front offset from it as if it were exact.

   crates/perry-runtime/src/arena/allocators.rs:174:

   let pad = align.max(8);
   let total = (GC_HEADER_SIZE + size + pad - 1) & !(pad - 1);   // aligned UP
   ...
   (*header).size = total as u32;

   #10077 encodes the front offset implicitly, as the difference between the physical capacity recovered from that size and the remaining capacity:

   pub(crate) unsafe fn array_physical_capacity(arr: *const ArrayHeader) -> usize {
       let header = (arr as *const u8).sub(GC_HEADER_SIZE) as *const GcHeader;
       ((*header).size as usize - GC_HEADER_SIZE - size_of::<ArrayHeader>()) / 8
   }
   pub(crate) unsafe fn array_front_offset(arr: *const ArrayHeader) -> usize {
       array_physical_capacity(arr) - (*arr).capacity as usize
   }

   Whenever the allocation was padded, array_physical_capacity overestimates, so an array that was never shifted reports a phantom front offset and every element read lands past true element zero. storage.rs's own header says "the GC header already records the allocation's byte size, so subtracting the remaining capacity from its physical capacity recovers the offset" — that holds only if the recorded size is exact, and it is not.

   How it was isolated

   Each row is a full run_parity_tests.sh build of test_gap_gc_http2_pending_event_callback_rooting on main (ea3a091b10):

   change	result
   none (current main)	CRASH (hang, 0 output, 10 s timeout)
   runtime array_front_offset → 0	CRASH
   codegen array_elements_addr → array+8	CRASH
   ext js_array_get → direct +8 read, both files	PASS
   ext, server/types.rs only	PASS
   runtime → 0 AND codegen → array+8 together	PASS

   The single-sided neutralisations both fail because there are two independent readers of the offset — the runtime function and the IR emitted by LlBlock::array_elements_addr — and fixing one leaves the other wrong. Neutralising both together is what passes, which is what identifies the derivation rather than any one call site.

   The ext rows pass because a raw +8 read bypasses the offset entirely; that is why parse_listen_args stops recognising the listen callback on main (the HTTP server never signals ready, so the test hangs with no output rather than failing an assertion).

   Correcting two earlier comments on this issue

   • My first hypothesis — the unguarded header read in gc_element_slot_range — was wrong; I tested and retracted it above.
   • I also suggested the ext accessor swap itself was the defect. That is also wrong: it is a symptom. I built an offset-aware raw-bits FFI accessor (js_array_raw_element_bits, reading at array_elements_ptr) and the test still crashed, because the offset it applies is the phantom one. Only bypassing the offset passes.

   Why a fix needs a design decision

   Encoding the offset as physical − capacity requires the recorded size to be exact, which the arena does not guarantee. Options, none of which a merge should pick unilaterally:

   1. store the front offset explicitly in ArrayHeader (costs a word, removes the inference entirely);
   2. record the exact requested size alongside the padded one;
   3. guarantee array allocations are never padded (align 8 and size a multiple of 8 on every path — arena_alloc_longlived / arena_alloc_old included) and assert it.

   Happy to implement whichever is preferred. Worth noting the blast radius is wider than this one test: every indexed array read in both the runtime and generated code goes through this offset, so any padded array is affected — this test is just the one that fails loudly.
3. proggeramlug

   changed the title [-]GC: test_gap_gc_http2_pending_event_callback_rooting hangs on main since #10077 (gc_element_slot_range can return None from an unguarded header read)[/-] [+]GC/array: #10077's queue front offset is derived from the PADDED GcHeader.size, so unshifted arrays report a phantom offset (hangs test_gap_gc_http2_pending_event_callback_rooting)[/+] on Sep 12, 2026
4. proggeramlug commented on Sep 12, 2026

   

   proggeramlug

   on Sep 12, 2026

   ContributorAuthor

   More actions

   Correction: retract the "padded GcHeader.size" explanation above. It is not established.

   I posted that from reading the padding in arena_alloc_longlived/arena_alloc_old:

   let pad = align.max(8);
   let total = (GC_HEADER_SIZE + size + pad - 1) & !(pad - 1);

   without checking that ordinary arrays take that path. They do not. js_array_alloc calls arena_alloc_gc(array_byte_size(actual_capacity), 8, GC_TYPE_ARRAY), and with GC_HEADER_SIZE == 8, size_of::<ArrayHeader>() == 8 and eight-byte elements the total is already a multiple of 8 — so pad = 8 rounds nothing. It also sets (*ptr).capacity = actual_capacity, the same value the allocation was sized for, so physical_capacity - capacity == 0 there. Third time I have been wrong about the mechanism on this issue; posting it rather than quietly deleting it.

   What remains established, purely empirically — each row a full run_parity_tests.sh build on main ea3a091b10:

   change	result
   none	CRASH (hang, 0 output, 10 s timeout)
   runtime array_front_offset → 0	CRASH
   codegen array_elements_addr → array+8	CRASH
   ext js_array_get → raw +8 read (server/types.rs alone suffices)	PASS
   runtime → 0 AND codegen → array+8 together	PASS

   Two conclusions I am confident in:

   1. The front offset is nonzero for at least one array on this path, and that is the mechanism. Bypassing it passes; applying it crashes. I also built an offset-aware raw-bits FFI accessor (js_array_raw_element_bits, reading at array_elements_ptr and skipping js_array_get's JS-facing resolution) — it still crashed, because it faithfully applies the offset. Only bypassing it works.
   2. There are two independent readers — the runtime function and the IR from LlBlock::array_elements_addr — so neutralising either alone leaves the other wrong. That is why the single-sided diagnostics both failed, and it is a hazard in its own right for anyone fixing this.

   What is not established: why the offset is nonzero. The array reaching parse_listen_args is a codegen rest-ABI argument array, which may not come from js_array_alloc. The next step is to measure rather than reason — instrument array_front_offset to report the array and the offset when it is nonzero — and I will follow up with that.
5. proggeramlug

   changed the title [-]GC/array: #10077's queue front offset is derived from the PADDED GcHeader.size, so unshifted arrays report a phantom offset (hangs test_gap_gc_http2_pending_event_callback_rooting)[/-] [+]GC/array: #10077's dense-queue front offset is nonzero for an unshifted array, hanging test_gap_gc_http2_pending_event_callback_rooting (two independent readers)[/+] on Sep 12, 2026
6. proggeramlug commented on Sep 12, 2026

   

   proggeramlug

   on Sep 12, 2026

   ContributorAuthor

   More actions

   Where I am stopping, and what is actually verified

   Reproducibility settled first, since everything below depends on it. Same build, repeated:

   • current main (ea3a091b10): CRASH 4/4 (and 3/3 in an earlier session) — deterministic, not flaky.
   • main with parse_listen_args reading elements directly instead of through perry_ffi::js_array_get: PASS 3/3.

   So there is one reproducible mitigation:

   // crates/perry-ext-http/src/server/types.rs, parse_listen_args
   -let bits = perry_ffi::js_array_get(arr_ptr, i as u32).bits();
   +let bits = *((arr_ptr as *const u8).add(8) as *const u64).add(i);

   That is not a fix to propose as-is — the raw +8 is exactly what #10077 set out to remove, and it would reintroduce the shifted-array bug for a genuinely shifted queue. It is a bisect result: it localises the failure to that one read.

   Verified frame

   tree	result
   before the 2026-09-12 trains (435d6396c)	PASS 2/2
   + train161 (c58c1fae5)	PASS
   + train162 (dc0d876fe)	PASS
   + all of #10075 (a816fd2e7)	PASS
   + all of #10077 (9b518346a)	CRASH
   current main	CRASH 4/4

   #10077 introduces it; #10075 does not; the maintainer commit above them does not.

   Two independent readers of the front offset. The runtime's array_front_offset and the IR from LlBlock::array_elements_addr compute it separately. Neutralising either alone leaves the other live — both single-sided diagnostics still crashed, and only neutralising both passed. Anyone fixing this should expect a one-sided change to appear to do nothing.

   Mechanisms I proposed and then disproved — please do not re-spend time on these

   1. Unguarded header read in gc_element_slot_range returning None (so the collector scans no element slots). Guarded it behind try_read_tracked_gc_header; compiles, hang persists.
   2. The ext accessor swap as the root cause rather than a symptom. Built an offset-aware raw-bits accessor (js_array_raw_element_bits, reading at array_elements_ptr, skipping js_array_get's JS-facing resolution); still crashes. Only bypassing the offset entirely passes.
   3. Allocator padding inflating GcHeader.size. js_array_alloc uses align 8 with a size already a multiple of 8 and sets capacity to the value it sized for, so nothing is rounded on that path.
   4. Codegen reading the wrong header field. GcHeader is #[repr(C)] with size at header+4, which is exactly the array - 4 codegen loads; capacity at array + 4 is right too.
   5. "The runtime offset is always 0." I instrumented array_front_offset to log every nonzero result and got nothing — but that is a null result, not a measurement: the function is simply not reached on this path, because js_array_get gets to elements another way.

   What I would look at next

   The failing read is js_array_get → js_array_get_f64 on a codegen rest-ABI argument array. That accessor does pointer cleaning, typed-array and buffer registry probes and descriptor gates; one of those, on an argument array that is not an ordinary dense heap array, is where the behaviour diverges from a plain bit read. Instrumenting js_array_get_f64 for this receiver — which branch it takes, and what it returns versus the stored bits — should settle it in one run.

   I have not fixed it and am not guessing further; the audit trail above is what I can stand behind.
7. proggeramlug commented on Sep 14, 2026

   

   proggeramlug

   on Sep 14, 2026

   ContributorAuthor

   More actions

   Fixed by merge train 186r (#10247) at eb13fa1.
8. proggeramlug

   closed this as completedon Sep 14, 2026