bug(codegen): custom property on a function object reads back undefined (fn.errors) #1802

Activity

1. proggeramlug

   added

   enhancementNew capability or improvementNew capability or improvement

   on May 25, 2026
2. proggeramlug commented on May 25, 2026

   

   proggeramlug

   on May 25, 2026

   ContributorAuthor

   More actions

   Root cause (pinpointed)

   The closure dynamic-property mechanism exists and works — closure_set_dynamic_prop / closure_get_dynamic_prop in crates/perry-runtime/src/closure/dynamic_props.rs, backed by a HashMap<usize /*closure addr*/, HashMap<String, f64>> side-table, wired into js_object_get/set_field_by_name. The bug is GC-correctness, not missing support.

   Minimal matrix (fn with a custom prop):

   case	value	read	perry
   A	number	later	✅ works
   B	[{msg}] set inside body	later	❌ undefined
   C	[{msg}] set top-level	later	❌ undefined
   D	[{msg}]	within same execution	✅ works

   Two GC problems with the side-table:

   1. Keyed by the closure's heap address. When subsequent allocation (e.g. the [{...}]) triggers a GC that moves/evacuates the closure, its address changes, so the lookup by the new address misses → undefined. (Number-valued A survives because it reads back before a move; D reads before any GC.)
   2. Stored pointer values aren't GC roots. Even without a move, an array/object stored only in the side-table can be collected since the side-table isn't scanned.

   So validate.errors (an array) set by an ajv standalone validator is lost after the next allocation/GC — which is exactly the ajv .errors symptom in #1680.

   Fix direction

   Make the side-table move-safe and its values rooted. Options: key by a stable closure id instead of raw address (and update on move), or register the side-table as a GC scanner that (a) rewrites closure keys on evacuation and (b) marks stored pointer values. This is a GC-sensitive change — medium/large, needs care around the generational evacuation path.
3. proggeramlug

   added a commit that references this issue on May 26, 2026

   feat(compiler): self-hosted build-time precompile() — Perry runs its …

   ...

   25f61b8
4. proggeramlug commented on May 26, 2026

   

   proggeramlug

   on May 26, 2026

   ContributorAuthor

   More actions

   Refined finding (re-verified on current main)

   Still reproduces: (f).errors = [{msg}] then reading (f).errors returns undefined (number-valued props work). Important correction to my earlier note: this is not simply "no GC support" — the machinery is fully wired:

   • gc/mod.rs:709 registers scan_closure_dynamic_props_roots_mut as a mutable root scanner (marks + rewrites the stored values),
   • gc/types.rs:419 calls closure_dynamic_props_owner_moved(old, new) to re-key on closure evacuation,
   • gc/layout.rs:1035 visits value slots for rewrite, and there's a write barrier on store.

   Yet a pointer-valued prop is lost while a number-valued prop survives, even in a tiny program. Since the read returns a clean undefined (entry gone, not a dangling pointer), the side-table key is being lost — consistent with the owner closure being moved/tenured by a minor GC triggered by the array allocation, and the re-key (closure_dynamic_props_owner_moved) producing an owner address that doesn't match the address used at the read site (or not firing for this closure shape). Number props survive because no pointer is involved (and/or less GC pressure).

   Next step is GC tracing (PERRY_GC_DIAG=1, and lldb on the re-key path) to confirm whether the move hook fires for these closures and whether old/new owner addresses line up with the get/set sites. This is a focused GC-evacuation debugging task in a historically fragile subsystem — not a quick fix.
5. proggeramlug commented on May 26, 2026

   

   proggeramlug

   on May 26, 2026

   ContributorAuthor

   More actions

   Full root-cause (investigated end-to-end)

   The corruption is not a closure-property storage bug and not about closure movement — it's that closure dynamic-property values are never marked/traced during GC, so a pointer-valued prop's nested contents get collected. Disambiguated repro:

   function fb(){ (fb as any).errors = [{ msg: "x" }]; }
   fb();
   gc();
   const e = (fb as any).errors;        // e is a valid array, length 1 …
   console.log(typeof e[0], e[0]?.msg); // … but e[0] is "string"/undefined (node: object/"x")

   The array survives (length 1) but its inner object is swept; e[0] reads a stale slot.

   Why — the precise gap (vs the working Object-overflow analog)

   Object overflow fields (the identical external-side-table pattern) survive GC because they register two scanners:

   • scan_overflow_fields_roots(mark) — non-mutable mark/keep-alive (RuntimeRootVisitor::for_copy(mark)), and
   • scan_overflow_fields_roots_mut — mutable rewrite,

   plus the owner object copies its overflow slots during minor GC via the rewrite descriptor (gc/layout.rs:1029).

   Closure dynamic props register only scan_closure_dynamic_props_roots_mut (mutable), and it early-returns unless is_metadata_rewrite_phase() — so values are never marked in the Mark or Copy phases. The rewrite descriptor visits closure-prop slots (gc/layout.rs:1035) but rewrite only follows already-forwarded pointers; it can't keep an otherwise-unreachable value alive.

   Why the naive fix (remove the early-return) is wrong

   It makes the mutable/evacuation scanner mark in the copying minor-GC (CopyingMark) phase → SIGSEGV (exit 138) under default gen-GC and PERRY_GEN_GC_EVACUATE=0; under PERRY_GEN_GC=0 (mark-sweep) it doesn't crash but still doesn't trace (e[0] stays string). Overflow avoids the crash because its minor-GC keep-alive flows through the owner-object copy, not the mutable root scanner.

   Correct fix (careful, multi-path)

   Mirror overflow exactly: add a non-mutable keep-alive scanner for closure props (scan_closure_dynamic_props_roots(mark) via for_copy, registered with gc_register_root_scanner) for the full-mark-sweep path, and ensure the copying minor-GC traces the prop values when the owner closure is copied (a mark/copy analog of the layout.rs:1035 rewrite hook), not the evacuation scanner. Verify with PERRY_GC_VERIFY_EVACUATION=1 across default / GEN_GC=0 / GEN_GC_EVACUATE=0. This is focused, instrumented GC work — the early-return is load-bearing and must not simply be removed.
6. proggeramlug commented on May 26, 2026

   

   proggeramlug

   on May 26, 2026

   ContributorAuthor

   More actions

   Self-contained handoff prompt (paste to a fresh agent)

   Fix GitHub issue #1802 in the Perry compiler (Rust; TypeScript → native via LLVM). Work in a git worktree branched from latest main.

   What's broken: closure dynamic-property values aren't GC-marked, so a pointer-valued property on a function object (e.g. ajv standalone's validate.errors = [...]) loses its nested objects after a GC — errors[0] reads back as a stale string. The full diagnosis, exact repro, root cause, the failed naive approach, the fix template, verification matrix, and key file/line refs are in the issue body — run gh issue view 1802 and read it completely before touching code.

   Hard constraints (learned the hard way):

   • The naive fix — removing the if !is_metadata_rewrite_phase() { return; } early-return in scan_closure_dynamic_props_roots_mut — SIGSEGVs in the copying minor-GC. The early-return is load-bearing. Get a crash backtrace first: build with PERRY_DEBUG_SYMBOLS=1, run the repro binary under lldb, bt at the fault. Don't guess.
   • Mirror the working Object-overflow-fields analog (crates/perry-runtime/src/object/mod.rs::scan_overflow_fields_roots_mut), and handle both the full mark-sweep path and the copying minor-GC path (trace prop values when the owner closure is copied, not via the evacuation root scanner).
   • Runtime change ⇒ rebuild runtime+stdlib+compiler: cargo build --release -p perry-runtime -p perry-stdlib -p perry. Compile test programs with PERRY_NO_AUTO_OPTIMIZE=1 so the freshly-built static runtime is actually linked (otherwise you'll test a stale lib).
   • Verify across every GC mode — no crash, correct value — with the repro below:
     default gen-GC, PERRY_GEN_GC=0, PERRY_GEN_GC_EVACUATE=0, PERRY_GC_FORCE_EVACUATE=1, and PERRY_GC_VERIFY_EVACUATION=1 (must not panic).

   Repro (must print object x):

   function fb() { (fb as any).errors = [{ msg: "x" }]; }
   fb(); gc();
   const e = (fb as any).errors;
   console.log(typeof e[0], e[0]?.msg);

   Deliverables: the fix + a test-files/ parity test (node-comparable; functions can carry own properties), cargo fmt --all -- --check clean, cargo clippy clean, and a PR to main. Do not bump the workspace version or edit CHANGELOG.md (the maintainer folds those in at merge). Do not git add -A in a worktree (it stages the target symlink + CWD artifacts). Closes #1802; unblocks #1803 / the fast-json-stringify path under #1677.
7. proggeramlug

   added a commit that references this issue on May 28, 2026

   fix(runtime): trace closure dynamic-prop values in all GC phases (#1802…

   ...

   13b8310
8. proggeramlug commented on May 28, 2026

   

   proggeramlug

   on May 28, 2026

   ContributorAuthor

   More actions

   Fixed in #2284 — scan_closure_dynamic_props_roots_mut now traces values in every GC phase, matching scan_overflow_fields_roots_mut.
9. proggeramlug

   closed this as completedon May 28, 2026