Skip to content

A closure-captured array local reads undefined in its declaring scope once Array.prototype has had an indexed property (merge train 219 regression; blocks pr-gate shard 4) #10727

Closed

Description

@proggeramlug

Read this first. The fixture is named test_gap_array_side_mask_covers_a_pointer_stored_at_a_late_index and its companions live in gc/tests/. It is not a GC bug, not about side masks, not about late indices, and not array corruption. The array is fine. What breaks is the binding: two storages for one captured local disagree, so the declaring scope reads undefined while a closure over the same variable still returns the correct array. Anyone reading the fixture name goes to array/; the defect is in capture lowering.

The bug

// 1. arm the array-prototype index deopt, then remove the property again
Object.defineProperty(Array.prototype, "11", { configurable: true, get() { return "P"; } });
delete (Array.prototype as any)[11];

function z4(): void {
  const dest: any[] = new Array(12);
  dest[0] = { old: "O" };
  for (let i = 1; i < 12; i++) { dest[i] = i; }
  function S(): any[] { return dest; }        // <-- nested closure capturing `dest`
  console.log(typeof dest, S().length, S() === dest);
}
z4();

Node prints object 12 true. Perry prints undefined 12 false.

An array local captured by a nested closure reads back undefined in its own declaring scope, once ARRAY_PROTO_HAS_INDEX has ever been armed. The closure still sees the correct array.

S() returns a correct 12-element array; dest reads undefined; S() === dest is false. The array was never damaged — the two storages for the binding diverged.

The latch (array/indexing_support.rs:99) is monotone set-only: a single Object.defineProperty(Array.prototype, <index>, …) anywhere in the process arms it permanently, and deleting the property does not disarm it.

This is reachable from ordinary code

An array local captured by a closure is not an exotic shape — it is one of the most common patterns in JavaScript. The only reason this took so long to surface is the trigger: something must have put an indexed property on Array.prototype at some point in the process. The trigger is rare; the affected pattern is not. Any program that does both is exposed, and the symptom is a silently empty-looking variable rather than a crash.

Per-read behaviour

scenario() called once before and once after arming, same binary:

read before arm after arm
typeof dest object undefined
dest === undefined false true
Array.isArray(dest) true false
dest.length 12 throws Cannot read properties of undefined
dest[0] [object Object] undefined
typeof S() object object
S().length 12 12
S() === dest true false

Controls

Both ingredients required; neither alone does anything. PERRY_NO_AUTO_OPTIMIZE=1 throughout, so this is not the specializer.

case result
latch armed, build only, no closure correct
latch armed, build + sparse array literal, no closure correct
latch armed, build + dense array literal, no closure correct
latch armed, build + capturing closure broken
latch not armed, build + capturing closure correct
latch not armed, everything incl. Symbol.species + slice correct

Acceptance criteria for a fix

  1. The reproducer above prints object 12 true.
  2. It must explain the hoisting inversion. --trace hir shows S hoisted above dest's Let (correct JS for a function declaration), so the capturing closure is created before the binding it captures is initialised — yet the hoisted S resolves correctly, while reads in closures created after the Let read undefined. Any proposed fix that does not account for that asymmetry is incomplete.
  3. test_gap_array_side_mask_covers_a_pointer_stored_at_a_late_index passes without re-baselining gap_snapshot.json.

HIR

[0] PreallocateBoxes([4, 5])
[1] Let { id: 4, name: "S",    init: Closure { captures: [5], body: [Return(LocalGet(5))] } }
[2] Let { id: 5, name: "dest", ty: Array(Any), init: New { class_name: "Array", args: [Integer(12)] } }
[3] Expr(PutValueSet { target: LocalGet(5), key: Integer(0), value: New{...}, receiver: LocalGet(5) })
[4] For { ... body: [Expr(PutValueSet { target: LocalGet(5), key: LocalGet(7), value: LocalGet(7) })] }

Candidate site — not yet a conclusion

crates/perry-codegen/src/expr/index_set.rs, emit_array_runtime_key_store (~line 409). After the runtime store returns the live (possibly reallocated) head:

if let Expr::LocalGet(id) = object {
    if let Some(slot) = ctx.locals.get(id).cloned() { /* plain local */ }
    else if let Some(global_name) = ctx.module_globals.get(id).cloned() { /* module global */ }
}

Two cases, and no case for a boxed/captured local. This path is reached only via the generic runtime-key route, which is exactly what the armed latch forces — so the two-ingredient dependency falls out of the code rather than being fitted to it. Still labelled a candidate: which storage each read lowers against has not been confirmed.

Gate impact

Absent from test-parity/gap_snapshot.json, so it blocks pr-gate. At 4715bc2fa1 (870 test_gap_ fixtures) the fixture is at index 351 → shard 4 of 6 (PR tier), 1 of 3 (sweep), 8 of 8 (full). Recompute with: find test-files -maxdepth 1 -type f \( -name '*.ts' -o -name '*.cts' -o -name '*.mts' \) | sort, keep basenames containing test_gap_, 0-based index k; shard i of M runs k % M == i - 1.

Observed failing at b0af11e1ce in two modes (fast 3-shard 3, auto-optimize 8-shard 7) and at 4715bc2fa1 (sweep shard 1, plus a local run); passing at 8df83f8c12. Whether the underlying defect predates train 219 or regressed there is being settled by a build at train 218; the title will be corrected once that lands.

How it presents through the fixture

The fixture installs a throwing getter on Array.prototype[11] (arming the latch) and holds destination in a SpeciesResult closure. First invocation: latch not yet armed, passes. Second: destination reads undefined and destination[10].label throws. It is therefore not slice-vs-splice (both fail; slice passing first is an ordering artefact), and the value is already wrong before the gc() call.

Retractions

Earlier comments reached conclusions later measurements overturned. Each was correct on its evidence at the time; all are superseded, and are left in place rather than edited so the path stays legible.

  1. The collector-mode matrix measured the wrong thing. The value is wrong before any collection, so the collector was never involved.
  2. "An array built while the latch is armed" was over-general. A bare build under an armed latch is fine. The missing conjunct was the closure capture.
  3. One probe supporting that discriminator was ill-formed, and Node fails it too. It tried to create an own index 11 while a setter-less accessor sat at Array.prototype[11], which no engine permits. I was measuring my own bug.
  4. The two "separate parity bugs" reported earlier were also my probe's fault, not Perry's. A clean install/delete cycle round-trips correctly and Object.getOwnPropertyDescriptor(Array.prototype, "11") returns undefined, matching Node. Those issues will not be filed.
  5. "The array is born empty" was one level too shallow. The array is intact; the binding diverged.

One elimination still stands: src_exotic was never the divergence, since splice's predicate is strictly broader than slice's.


Footnote for anyone automating against the Actions API. A job still running reports conclusion: "", not conclusion: null. A watcher written as select(.conclusion == null) matches nothing and reports "done" on the first poll — a pass indistinguishable from never having run. Gate on .status != "completed".

Activity

  1. proggeramlug commented on Sep 19, 2026

    @proggeramlug
    ContributorAuthor

    Reproduced locally at 4715bc2fa1, with the actual diff

    Release build of main @ 4715bc2fa1, macOS, harness invoked exactly as CI's fast mode does (PERRY_SKIP_BUILD=1 PERRY_BIN=…). Reproduces in 15 s.

    FAIL  test_gap_array_side_mask_covers_a_pointer_stored_at_a_late_index (output mismatch)
           Node exit:   0
           Perry exit:  1
    

    Raw outputs:

    --- node --experimental-strip-types            --- perry (exit 1)
    slice:slice-stop:slice-late                    slice:slice-stop:slice-late
    splice:splice-stop:splice-late                 <nothing>
    

    Perry's stderr:

    TypeError: Cannot read properties of undefined (reading 'label')
        at array_side_mask_covers_a_pointer_stored_at_a_late_index (<anonymous>)
    

    slice passes; only splice fails. The two halves are the same function with one parameter changed, so whatever differs is specific to the splice path.

    The slot is actively cleared, not merely left unwritten

    Worth stating because it rules out the boring explanation. destination is pre-filled by the fixture's own init loop (destination[i] = i for i in 1..11), so index 10 holds the number 10 before the operation runs.

    If Perry's splice had simply never written index 10 — aborting at the throwing index-11 getter before reaching it — the slot would still read 10, and (10).label is undefined without throwing, printing splice:splice-stop:undefined.

    Instead we get a TypeError on reading .label of undefined, so destination[10] is itself undefined. Something changed that slot from 10 to undefined. That is consistent with the pointer having been written and then lost, and not with the operation skipping the slot.

    Collector-mode matrix — deterministic and mode-independent

    Same binary, no rebuild between arms:

    arm runs result
    default 5 fail, 5/5, byte-identical
    PERRY_GEN_GC=0 (full mark-sweep, non-moving) 3 fail, 3/3
    PERRY_GC_FORCE_EVACUATE=1 3 fail, 3/3

    Failing under PERRY_GEN_GC=0 does not exonerate the collector: both collectors consult the same side mask to decide what to trace, and only the moving one additionally rewrites. A mask that does not cover index 10 loses the object under either — non-moving by collecting it, moving by collecting or failing to rewrite it. So this is consistent with the tracing/retention bug the fixture was written for.

    What the matrix does rule out is timing. A heap-layout or allocation-placement perturbation would show up as flaky or mode-sensitive; 11/11 byte-identical failures across three collector configurations says a functional defect, not a scheduling accident.

    This reorders the candidate list

    My earlier ordering put #10555 (7c45dc94c5) first on the theory that installing ~15 Symbol.toStringTag descriptors at startup shifts allocation placement and collection timing. The determinism above is evidence against that, since that mechanism predicts exactly the timing sensitivity we do not observe. Demoted.

    Revised order for the A/B:

    1. instanceof against a ClassExprFresh parent resolves by shared class id, so an earlier evaluation's class can fail instanceof after a later evaluation #10624 / 4f73a760f4 — now first. It pins a hidden INSTANCE_CONSTRUCTING_CLASS_KEY onto newly constructed instances and edits object/field_get_set/enumeration.rs. ArraySpeciesCreate reaches the species callback via new, so if that construction path pins a hidden named key onto the returned array, an array acquiring a named property can change its representation and invalidate or bypass the side mask. That is a functional, deterministic mechanism, which is the shape the evidence now calls for.
    2. Web/runtime built-ins lack Symbol.toStringTag: Object.prototype.toString.call(new URLSearchParams()) is [object Object] (URL, Headers, Blob, Request, Response, FormData, TextEncoder, AbortController, Event, EventTarget…) #10555 / 7c45dc94c5 — demoted, as above.
    3. fix(runtime): root event/listener dispatch copies across moving GC #10606 / 57506478c0 — last. Its changed code is only reachable from event/stream/timer/domain/worker dispatch and this fixture contains none of those, so it never executes here.

    Still hypotheses; the A/B is what settles it. All three are runtime/stdlib-only, so each hop is an incremental rebuild rather than a full compiler build.

  2. proggeramlug commented on Sep 19, 2026

    @proggeramlug
    ContributorAuthor

    This is not a GC bug, and it is not slice-vs-splice

    Four instrumented fixture compiles against the same 4715bc2fa1 binary (no rebuild). Three findings, each of which contradicts the framing this issue was opened with — including mine.

    1. The slot is already wrong BEFORE the collection

    Reading destination[10] immediately after the operation and again after gc():

    slice   caught=slice-stop   PRE =object(label=slice-late)  POST=object(label=slice-late)   <- ok
    splice  caught=splice-stop  PRE =undefined                 POST=undefined                  <- bad BEFORE gc
    

    PRE is already undefined. The collection is irrelevant. And late itself is alive and well throughout (lateStillOk=object(label=splice-late)), so nothing was collected — the value was never stored.

    The fixture is named array_side_mask_covers_a_pointer_stored_at_a_late_index and its companions live in gc/tests/, which is why it reads as a tracing/retention failure. It is a functional bug in the slice/splice species-copy path. Any GC framing — including the collector-mode matrix in my previous comment, which was measuring the wrong thing — should be set aside.

    2. slice fails too

    Adding more invocations to the same process:

    slice        PRE=object(label=slice-late)   <- passes
    splice       PRE=undefined
    slice+keep   PRE=undefined                  <- same method, now fails
    splice+keep  PRE=undefined
    

    Only the first invocation passes. The real fixture's "slice passes, splice fails" is an artefact of slice running first. Both methods are equally affected.

    3. The discriminator is the latch state when the source array is BUILT

    Not the call ordinal, not the method. A third probe that builds the arrays after installing the getter fails on every call including the first; one that builds them before the first-ever install passes.

    The consistent rule across all three probes:

    An array built while ARRAY_PROTO_HAS_INDEX is already true loses its late-index element in the species copy.

    That latch (array/indexing_support.rs:99) is monotone set-only, armed by the first indexed write to Array.prototype. So in the real fixture, invocation 1 (slice) builds its arrays with the latch still false and passes; invocation 2 (splice) builds them with the latch armed by invocation 1 and fails. Exactly the observed pattern, and nothing to do with which method is called.

    Mechanism

    In the exotic branch of both methods (array/splice_slice.rs), each element is fetched via array_spec_has_index / array_spec_get and stored with note_array_slot. If array_spec_has_index(source, 10) returns false, v becomes TAG_HOLE and note_array_slot writes a hole, which reads back as undefined — precisely the symptom.

    The probe shows the source is otherwise intact: source[10] === late is true and Object.prototype.hasOwnProperty.call(source, 10) is true. So this is an inconsistency between array_spec_has_index and ordinary indexed reads, for arrays built while the latch is armed.

    Two eliminations from static reading, recorded so nobody re-derives them:

    • src_exotic is not the divergence. splice's predicate is array_iteration_is_exotic(arr) || array_static_proto_recorded() and slice's is just array_iteration_is_exotic(arr) — strictly broader, so splice cannot take the non-exotic path while slice takes the exotic one. Confirmed empirically: every run reports caught=...-stop, so the getter fires and the exotic path is taken in all of them.
    • ARRAY_PROTO_HAS_INDEX is never cleared. There is no store(false) anywhere in the tree. The appealing theory that the finally's delete Array.prototype[11] disarms it for the next invocation is false.

    Consequence for the A/B

    The three candidates queued for A/B (#10624, #10555, #10606) were all chosen on a GC-perturbation theory that finding 1 removes. None of them touches array indexing, so the ordering should be re-derived against the property-install / array-element-store path rather than the collector. The A/B itself is now much cheaper: the probe reproduces in a 3-second compile and fails on every invocation, so no full fixture run is needed per hop.

    Two separate parity bugs found in passing

    Both reproduce on 4715bc2fa1 and are unrelated to the above; filing separately if they are not already known.

    1. Object.getOwnPropertyDescriptor(Array.prototype, "11") returns null after the property has been deleted, where Node returns undefined.
    2. A second install/delete cycle on Array.prototype["11"] throws TypeError: Cannot convert undefined or null to object, i.e. the prototype is left unusable by the first cycle. Node completes five cycles without complaint.
  3. proggeramlug commented on Sep 19, 2026

    @proggeramlug
    ContributorAuthor

    Root cause found, and it is none of the things above. Correcting my own two previous comments.

    Minimal reproducer — 8 lines, no GC, no slice/splice, no Symbol.species, no repeat:

    // 1. arm the array-prototype index deopt, then remove the property again
    Object.defineProperty(Array.prototype, "11", { configurable: true, get() { return "P"; } });
    delete (Array.prototype as any)[11];
    
    function z4(): void {
      const dest: any[] = new Array(12);
      dest[0] = { old: "O" };
      for (let i = 1; i < 12; i++) { dest[i] = i; }
      function S(): any[] { return dest; }   // <-- a nested closure capturing `dest`
      console.log(dest[0], dest[5], dest[11]);
    }
    z4();

    Node prints the elements. Perry prints undefined for every slot. Remove the closure S and it is correct; keep it and the whole array reads empty.

    An array local captured by a nested closure reads back all undefined, once ARRAY_PROTO_HAS_INDEX has ever been armed.

    The latch (array/indexing_support.rs:99) is monotone set-only, so a single Object.defineProperty(Array.prototype, <index>, …) anywhere in the process arms it permanently — deleting the property does not disarm it. From that point on, any function that captures a local array in a closure sees an empty array.

    Controls, all on the same binary

    case result
    latch armed, build only, no closure correct
    latch armed, build + sparse array literal, no closure correct
    latch armed, build + dense array literal, no closure correct
    latch armed, build + capturing closure all undefined
    latch not armed, build + capturing closure correct
    latch not armed, everything incl. species + slice correct

    Both ingredients are required, and neither alone does anything.

    Why the fixture looked like a GC side-mask bug

    test_gap_array_side_mask_... installs the throwing getter (arming the latch) and holds destination in a SpeciesResult closure. On the first invocation the latch is not yet armed, so it passes; on the second the latch is armed by the first, destination is born empty, and destination[10].label throws. The fixture only inspects index 10, so a whole-array wipe presented as one lost pointer at a late index — and the name plus gc/tests/ placement made a collector bug the natural reading. That naming cost this investigation most of its time and is worth fixing alongside the bug.

    Corrections to my earlier comments in this thread

    Both were correct on their evidence at the time; both are now superseded.

    • The collector-mode matrix measured the wrong thing. destination[10] is already undefined before gc(), so the collector was never involved. The finding that stands from that comment is only the before-the-collection read, which is what broke the GC framing open.
    • The "array built while the latch is armed" discriminator I proposed was over-general: a bare build under an armed latch is fine (control row 1 above). The missing conjunct is the closure capture. One probe I used to support it was also ill-formed — it created an own index 11 while a setter-less accessor sat at Array.prototype[11], which Node rejects too, so that probe was measuring my own bug.
    • The src_exotic elimination still stands and is now moot: splice's predicate is strictly broader than slice's, so it could never have been the divergence, and slice-vs-splice turned out to be irrelevant anyway.
    • The two "separate parity bugs" I reported at the end of the second comment were also my probe's fault, not Perry's: a clean install/delete cycle round-trips correctly and getOwnPropertyDescriptor returns undefined, matching Node. Disregard them; I will not be filing those issues.

    Where the defect is likely to live

    Not in splice_slice.rs, and not in the collector. The interaction is between the closure-capture storage of an array local and the armed-latch (prototype-aware) array element path — i.e. perry-codegen's capture lowering and/or array/indexing*.rs's deopt path resolving the capture cell rather than the array it holds. PERRY_NO_AUTO_OPTIMIZE=1 is set throughout, so this is not the specializer.

  4. proggeramlug commented on Sep 19, 2026

    @proggeramlug
    ContributorAuthor

    Sharper characterisation: the box is correct, the declaring scope is not

    Per-read probe, same binary, scenario() called once before and once after arming the latch:

    read before arm after arm
    typeof dest object undefined
    dest === undefined false true
    Array.isArray(dest) true false
    dest.length 12 throws Cannot read properties of undefined
    dest[0] [object Object] undefined
    typeof S() object object
    S().length 12 12
    S() === dest true false

    So the array is intact and the closure still reaches it — S() returns a correct 12-element array. What is lost is the binding in the declaring scope: dest itself reads undefined, and S() === dest is false because the two are no longer the same storage.

    This is a binding/capture-storage divergence, not array corruption. The earlier "the array is born empty" phrasing was one level too shallow, though its practical consequence is the same.

    HIR evidence

    --trace hir --focus scenario on the reproducer:

    [0] PreallocateBoxes([4, 5])
    [1] Let { id: 4, name: "S",    init: Closure { captures: [5], body: [Return(LocalGet(5))] } }
    [2] Let { id: 5, name: "dest", ty: Array(Any), init: New { class_name: "Array", args: [Integer(12)] } }
    [3] Expr(PutValueSet { target: LocalGet(5), key: Integer(0), value: New{...}, receiver: LocalGet(5) })
    [4] For { ... body: [Expr(PutValueSet { target: LocalGet(5), key: LocalGet(7), value: LocalGet(7) })] }
    

    Two things worth noting. dest (id 5) is box-allocated, as expected for a captured local. And S is hoisted above dest's Let — correct JS semantics for a function declaration, but it means the capturing closure is created before the binding it captures is initialised.

    The reads that go wrong are the ones in closures created after [2]; the hoisted S, created before it, still resolves correctly. That inversion is the most specific clue available.

    Candidate site

    crates/perry-codegen/src/expr/index_set.rs, emit_array_runtime_key_store (~line 409). After the runtime store returns the live (possibly reallocated) head, the writeback is:

    if let Expr::LocalGet(id) = object {
        if let Some(slot) = ctx.locals.get(id).cloned() {
            // store new head into the plain local slot
        } else if let Some(global_name) = ctx.module_globals.get(id).cloned() {
            // store new head into the module global
        }
    }

    Two cases, plain local and module global, and no case for a boxed/captured local. An array whose binding lives in a capture box therefore has a writeback target that either does not exist or is not the storage the rest of the function reads. This path is reached only when the store takes the generic runtime-key route, which is what the armed latch forces — matching the observed dependency on both ingredients.

    Stated as a candidate, not a conclusion: I have not yet confirmed which storage each read is lowered against, and this investigation has already overturned its own framing three times. The next step is a build at merge train 218 to establish whether the defect predates train 219, which decides whether this is revert-shaped or a months-latent correctness bug.

  5. changed the title [-]gap-suite: test_gap_array_side_mask_covers_a_pointer_stored_at_a_late_index regressed in merge train 219 (pass -> parity_fail), blocking pr-gate via shard 4[/-] [+]A closure-captured array local reads `undefined` in its declaring scope once `Array.prototype` has had an indexed property (merge train 219 regression; blocks pr-gate shard 4)[/+] on Sep 19, 2026
  6. proggeramlug commented on Sep 19, 2026

    @proggeramlug
    ContributorAuthor

    Dated: a merge train 219 regression, not pre-existing

    Built merge train 218 (60922041cd) from source in its own target dir — -p perry -p perry-runtime-static -p perry-stdlib-static in one invocation, so no stale archive can make the arms agree. Runtime stamp verified:

    PERRY_RUNTIME_BUILD_STAMP_V1|version=0.5.1596|build=git:60922041cd2fa7542baf22ad31ff4683e41a82eb
    

    Same two reproducers, same machine:

    check (after arming the latch) train 218 60922041cd train 220 4715bc2fa1
    typeof dest object undefined
    dest === undefined false true
    Array.isArray(dest) true false
    dest.length 12 throws
    dest[0] [object Object] undefined
    S() === dest true false
    z4 full slot dump all correct all undefined

    Train 218 is clean on the exact failing case. The defect entered in merge train 219 (8df83f8c12..b0af11e1ce) — 30 commits, ~13 touching code.

    This is the better of the two possibilities: the bug is revert-shaped rather than a months-latent defect, and a revert is preferable to a capture-lowering change going through a merge train.

    Bisect in progress

    The reproducer runs in 3 seconds, so build time dominates; ~4 hops worst case. Currently building at #16 of 30, 8cbf5bef09 "fix(hir): forward inherited captures through a locally-shadowed class parent" — near the midpoint and the strongest candidate on subject matter, since it edits perry-hir/src/lower/context.rs and lowering_context.rs, where capture/box decisions are made.

    Ordered candidates within the train, by how well they match "two storages for one captured local disagree":

    # commit why
    16 8cbf5bef09 fix(hir): forward inherited captures… edits capture lowering directly
    14 0ed806587c fix(codegen): refresh local_types on var redeclaration edits stmt/let_stmt.rs; the HIR evidence centres on a Let and its binding storage
    18 9c24b21ce8 fix(hir): reified receiver for a computed dynamic key HIR lowering of a receiver
    20 8844271cbe fix(transform): cross-module inlining sibling by value identity/storage of a bound value

    Note that none of these is the commit that fixed test_gap_10430 (57506478c0, #10606, still unproven as that fix) — the train both fixed one gap test and broke another, which is why the shard stayed red across the boundary and looked like a single continuing failure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions