From d6cbb12d931ae98f9b3514a8a5b343151e7d8d32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 30 Aug 2026 03:35:48 +0200 Subject: [PATCH 1/3] =?UTF-8?q?fix(runtime):=20closure-literal=20singleton?= =?UTF-8?q?=20caches=20conflated=20user=20function=20identity=20=E2=80=94?= =?UTF-8?q?=20pi=20boot=20threw=20Cyclic=20=5F=5Fproto=5F=5F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit perry-compiled pi (13MB esbuild bundle) died at startup with `TypeError: Cyclic __proto__ value` out of `js_object_set_prototype_of`, with obj_bits == proto_bits exactly — the two ARGUMENTS were the same pointer — while the chain behind the proto was healthy. None of the bundle's 17 textual `Object.setPrototypeOf(` sites fired a JS logging shim, because the self-set was manufactured upstream of the call: the closure-literal singleton caches handed back ONE ClosureHeader for two evaluations of the same function literal, so `setPrototypeOf(wrapped, original)` (a graceful-fs-style wrap pattern) received one object twice and correctly refused the "cycle". Mechanism: `expr/closure.rs` routed closure literals through `js_closure_alloc_singleton` (captureless arrows) and `js_closure_alloc_with_captures_singleton` (arrows with captures, and non-arrow literals whose captures are all boxes) keyed by (func_ptr, capture bits). Two evaluations of the same literal with bit-identical captures — e.g. an arrow capturing the same constant, or any captureless arrow — came back `===`-equal. ECMA-262 OrdinaryFunctionCreate requires a fresh object per evaluation, and the distinction is observable through `===`, expando properties, WeakMap keys, addEventListener de-duplication, and `Object.setPrototypeOf`. Minimal repros (byte-compared against node before/after): function mk() { return () => K; } // captured arrow const a = mk(), b = mk(); // perry: a === b (node: false) Object.setPrototypeOf(a, b); // perry threw Cyclic __proto__ and the same with `() => 1` (captureless). Both now match node. Fix: gate every closure.rs literal singleton path on `is_plain_async_step_body` — the file's existing detector for the compiler-synthesized plain-async step closures (their terminal `Stmt::ReleaseBoxes` arms cannot appear in user code). Those are the closures the caches were built for (#8269's parallel async-await pattern re-creates them per resume with the same per-activation box captures, and their identity never escapes the promise machinery), and they keep the fast path. Every user-authored arrow and function expression now mints a fresh closure. Runtime-internal singleton users (function-declaration references, property_get/i18n/arrays wrapper thunks) are separate paths and unchanged. A genuine `setPrototypeOf(x, x)` still throws — the cycle check is untouched. Perf note: this deliberately gives back the user-arrow closure reuse from the #8269/#8291 captured-singleton extension (e.g. ECS `World.executeEntityCommands`' per-call inner arrow) and the captureless user-arrow singleton at literal sites; a sound replacement needs escape-aware caching rather than identity-violating sharing. Validation: repros above and test-files/ test_gap_9090_closure_literal_identity.ts byte-identical to node; `cargo test -p perry-runtime --lib -- --test-threads=1` green — 2813 passed, 0 failed with `--skip reserved_floor` (that module's at-scale tests SIGABRT on this pre-#9110 base; known #9108/#9110, unrelated); `cargo test -p perry-codegen`: 283+75 passed after updating the four native_proof_regressions pins from `js_closure_alloc_singleton` to `js_closure_alloc` (their real subject — the alloc storing the public wrapper pointer — is preserved); one pre-existing env-leak flake (`packed_f64_loop_unary_math_store_versions_with_side_exit`) passes in isolation. Claude-Session: https://claude.ai/code/session_01Ay8VyLkKbm8Hkc1xmvTEsP --- crates/perry-codegen/src/expr/closure.rs | 40 ++++++++++++---- .../tests/native_proof_regressions.rs | 16 ++----- .../test_gap_9090_closure_literal_identity.ts | 46 +++++++++++++++++++ 3 files changed, 80 insertions(+), 22 deletions(-) create mode 100644 test-files/test_gap_9090_closure_literal_identity.ts diff --git a/crates/perry-codegen/src/expr/closure.rs b/crates/perry-codegen/src/expr/closure.rs index 9a22c66624..ce40282104 100644 --- a/crates/perry-codegen/src/expr/closure.rs +++ b/crates/perry-codegen/src/expr/closure.rs @@ -266,13 +266,12 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { // ran webhook_endpoints' method) — the `replace is not a function` // symptom. // - // To preserve the hot-path optimizations while restoring identity, - // arrow functions are singleton-eligible because they have no own - // `.prototype` and are not constructable. Compiler-synthesized - // non-arrow async callbacks whose captures are all boxes are also - // safe: they are never constructors and their cache key includes - // the box addresses. Other non-arrow closures are treated as - // potential constructors and always get a fresh instance. + // The Stripe fix restricted the caches to arrows (no own + // `.prototype`, not constructable) plus non-arrow all-boxed + // captures. pi's boot showed that is still not enough: identity + // itself is observable (`===`, expandos, setPrototypeOf), not + // just `.prototype`, so ANY user literal is off-limits — see the + // identity gate below. let mut write_ids = std::collections::HashSet::new(); crate::boxed_vars::collect_write_ids_in_stmts(body, &mut write_ids); let writes_unboxed_capture = auto_captures @@ -284,8 +283,30 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { && auto_captures .iter() .all(|cap_id| ctx.boxed_vars.contains(cap_id)); - let singleton_identity_safe = *is_arrow || captures_all_boxed; - let no_capture_singleton = *is_arrow && total_caps == 0; + // IDENTITY GATE (pi boot blocker): a closure literal evaluation + // must produce a FRESH function object every time it runs + // (ECMA-262 OrdinaryFunctionCreate). Sharing one ClosureHeader + // across evaluations is observable through `===`, expando + // properties, WeakMap/WeakSet keys, addEventListener identity + // de-duplication — and through `Object.setPrototypeOf(a, b)`, + // which for a conflated pair (a IS b) becomes a SELF-set and + // throws "TypeError: Cyclic __proto__ value". pi's esbuild + // bundle wires `setPrototypeOf(wrapped, original)` where both + // came from the same arrow literal with bit-identical captures, + // so its boot died on exactly that throw. + // + // The singleton caches therefore only serve closures the + // COMPILER synthesized: the async-activation step closures + // recognized by `is_plain_async_step_body` (their terminal + // `ReleaseBoxes` arms cannot appear in user code, and their + // identity never escapes the runtime's promise machinery). + // Those are the closures the caches were built for — re-created + // per resume with the same per-activation box captures. User + // arrows and function expressions always mint fresh objects. + let is_plain_async_step = is_plain_async_step_body(body); + let singleton_identity_safe = + is_plain_async_step && (*is_arrow || captures_all_boxed); + let no_capture_singleton = is_plain_async_step && *is_arrow && total_caps == 0; let captured_singleton = singleton_identity_safe && !no_capture_singleton && !writes_unboxed_capture; @@ -383,7 +404,6 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { // frame as escaped would delay every terminal cell until a full // GC. User closures nested inside it still take the dedicated // setter and therefore preserve #8213's escaped-cell lifetime. - let is_plain_async_step = is_plain_async_step_body(body); let boxed_capture_slots = auto_captures .iter() .map(|cap_id| ctx.boxed_vars.contains(cap_id)) diff --git a/crates/perry-codegen/tests/native_proof_regressions.rs b/crates/perry-codegen/tests/native_proof_regressions.rs index dd0c3673db..0df0ecb46f 100644 --- a/crates/perry-codegen/tests/native_proof_regressions.rs +++ b/crates/perry-codegen/tests/native_proof_regressions.rs @@ -12868,9 +12868,7 @@ fn typed_f64_closure_clone_emits_internal_clone_and_guarded_direct_call() { "typed closure should expose a public wrapper and keep an internal generic body:\n{ir}" ); assert!( - ir.contains(&format!( - "call i64 @js_closure_alloc_singleton(ptr @{public}" - )), + ir.contains(&format!("call i64 @js_closure_alloc(ptr @{public}")), "closure allocation must keep storing the public wrapper pointer:\n{ir}" ); assert!( @@ -12999,9 +12997,7 @@ fn typed_i32_closure_clone_emits_internal_clone_and_guarded_direct_call() { "typed-i32 closure should expose a public wrapper and keep an internal generic body:\n{ir}" ); assert!( - ir.contains(&format!( - "call i64 @js_closure_alloc_singleton(ptr @{public}" - )), + ir.contains(&format!("call i64 @js_closure_alloc(ptr @{public}")), "closure allocation must keep storing the public wrapper pointer:\n{ir}" ); assert!( @@ -13198,9 +13194,7 @@ fn typed_i1_closure_clone_emits_internal_clone_and_guarded_direct_call() { "typed closure should expose a public wrapper and keep an internal generic body:\n{ir}" ); assert!( - ir.contains(&format!( - "call i64 @js_closure_alloc_singleton(ptr @{public}" - )), + ir.contains(&format!("call i64 @js_closure_alloc(ptr @{public}")), "closure allocation must keep storing the public wrapper pointer:\n{ir}" ); assert!( @@ -13451,9 +13445,7 @@ fn typed_string_closure_clone_emits_internal_clone_and_guarded_direct_call() { "typed string closure should expose a public wrapper and keep an internal generic body:\n{ir}" ); assert!( - ir.contains(&format!( - "call i64 @js_closure_alloc_singleton(ptr @{public}" - )), + ir.contains(&format!("call i64 @js_closure_alloc(ptr @{public}")), "closure allocation must keep storing the public wrapper pointer:\n{ir}" ); assert!( diff --git a/test-files/test_gap_9090_closure_literal_identity.ts b/test-files/test_gap_9090_closure_literal_identity.ts new file mode 100644 index 0000000000..6dcf5a3c94 --- /dev/null +++ b/test-files/test_gap_9090_closure_literal_identity.ts @@ -0,0 +1,46 @@ +// Every evaluation of a closure literal creates a fresh function object +// (ECMA-262 OrdinaryFunctionCreate). The captured-closure singleton cache +// conflated two evaluations of the same literal whenever their capture bits +// matched — observable through `===`, and fatally through +// `Object.setPrototypeOf(a, b)`, which for a conflated pair is a SELF-set +// and throws "TypeError: Cyclic __proto__ value". pi's esbuild bundle died +// at boot on exactly that wiring (setPrototypeOf(wrapped, original) where +// both came from one arrow literal). + +// Case 1: arrow literal capturing the same constant value in both evals. +const K = { tag: 1 }; +function mkCaptured() { + return () => K; +} +const c1 = mkCaptured(); +const c2 = mkCaptured(); +console.log("captured-arrow distinct:", c1 !== c2); +Object.setPrototypeOf(c1, c2); +console.log("captured-arrow proto:", Object.getPrototypeOf(c1) === c2); + +// Case 2: captureless arrow literal. +function mkBare() { + return () => 1; +} +const b1 = mkBare(); +const b2 = mkBare(); +console.log("bare-arrow distinct:", b1 !== b2); +Object.setPrototypeOf(b1, b2); +console.log("bare-arrow proto:", Object.getPrototypeOf(b1) === b2); + +// Case 3: expandos must not alias across evaluations. +const e1: any = mkBare(); +const e2: any = mkBare(); +e1.mark = "one"; +e2.mark = "two"; +console.log("expando isolation:", e1.mark === "one" && e2.mark === "two"); + +// Case 4: a GENUINE self-set must still throw (the cycle check stays). +const solo: any = mkBare(); +let threw = false; +try { + Object.setPrototypeOf(solo, solo); +} catch (err: any) { + threw = err instanceof TypeError; +} +console.log("genuine self-set throws:", threw); From 28015b87b4fcb684885347c9f8e72a16866d7c1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 30 Aug 2026 04:55:47 +0200 Subject: [PATCH 2/3] =?UTF-8?q?fix(runtime):=20name-keyed=20builtin-member?= =?UTF-8?q?=20reads=20must=20see=20user=20overrides=20=E2=80=94=20pi=20boo?= =?UTF-8?q?t=20threw=20Cyclic=20=5F=5Fproto=5F=5F=20(part=202)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With the closure-literal identity fix in place, pi still died at startup with `TypeError: Cyclic __proto__ value`, obj_bits == proto_bits exactly. The instrumented throw site showed both arguments were ONE closure with `func_ptr = 0xBADD_DEAD` (BOUND_METHOD_FUNC_PTR, capture_count 3) and a healthy 3-link chain behind it — the canonical bound-native callable that `bound_native_callable_export_value` mints once per (module, member). The failing code is graceful-fs's module init, bundled into pi (pi-bundle.mjs:6621/6686/6705): var chdir = process.chdir; process.chdir = function (d) { ... }; if (Object.setPrototypeOf) Object.setPrototypeOf(process.chdir, chdir); and the same wrap for fs.rename / fs.read. Under perry the patch write did not round-trip on the re-read, so setPrototypeOf received the SAME canonical closure for both arguments — a self-set — and the cycle check correctly refused it. The earlier probe of this exact shape passed because it patched a PLAIN object, where writes round-trip; the failure needs a builtin namespace receiver. The JS shim over `Object.setPrototypeOf(` never fired because the conflation happens in the native member-READ, upstream of the call. Root cause: user writes to builtin namespace members are stored in two different places depending on the lowering — computed stores (`process[k] = fn`) go through `nm_field_set_override` into `NATIVE_NAMESPACE_PROP_OVERRIDES`, while static stores (`process.chdir = fn`) reach the generic store path and land as an own dynamic field on the canonical namespace object. The NAME-KEYED read entries carry no object pointer and consulted only the override table: * `js_native_module_property_by_name` (codegen static reads of process.* members) missed own-field stores, so the graceful-fs static patch was invisible to the static re-read; * `js_native_module_esm_export_value` (codegen property reads off a builtin DEFAULT import — `import fs from "node:fs"; fs.rename`) consulted NOTHING (consult_overrides=false plus its own snapshot cache), so no fs patch was ever visible. In Node the default import of a core module is the live mutable CJS exports object, so the patched value must win; the tls DEFAULT_* cache-coherence hack was the ad-hoc version of this for three keys. Fix: `native_namespace_user_value(module, prop)` consults the override table and then the canonical namespace object's own field (never creating a namespace — if none exists, no user store can have landed on one). Both name-keyed read entries call it before any built-in resolution or snapshot cache. Named ESM import bindings of core modules snapshot at module init before user patches run, so their intended snapshot semantics are unaffected in the eager case. Validation: r11-r16 probe matrix (process/fs, static/computed reads and writes) and test-files/test_gap_9091_native_member_patch_roundtrip.ts byte-identical to node; a genuine `setPrototypeOf(x, x)` still throws. Claude-Session: https://claude.ai/code/session_01Ay8VyLkKbm8Hkc1xmvTEsP --- .../perry-runtime/src/object/native_module.rs | 43 ++++++++++++++++++- ..._gap_9091_native_member_patch_roundtrip.ts | 34 +++++++++++++++ 2 files changed, 76 insertions(+), 1 deletion(-) create mode 100644 test-files/test_gap_9091_native_member_patch_roundtrip.ts diff --git a/crates/perry-runtime/src/object/native_module.rs b/crates/perry-runtime/src/object/native_module.rs index 927ae9bcc3..014de86012 100644 --- a/crates/perry-runtime/src/object/native_module.rs +++ b/crates/perry-runtime/src/object/native_module.rs @@ -153,6 +153,39 @@ pub(crate) fn native_namespace_prop_override_get(module: &str, prop: &str) -> Op }) } +/// pi boot blocker: a user write to a builtin namespace member must win every +/// subsequent NAME-KEYED read, no matter which lowering performed the store. +/// Today the stores are split: computed writes (`process[k] = fn`) land in +/// `NATIVE_NAMESPACE_PROP_OVERRIDES` via `nm_field_set_override`, while static +/// writes (`process.chdir = fn`) reach the generic store path and land as an +/// OWN dynamic field on the canonical namespace object. The name-keyed read +/// entries (`js_native_module_property_by_name`, +/// `js_native_module_esm_export_value`) carry no object pointer, so they only +/// consulted the override table — a static write was invisible to them and +/// the read handed back the canonical BOUND_METHOD closure again. graceful-fs +/// then did `Object.setPrototypeOf(process.chdir, chdir)` with the SAME +/// closure on both sides and pi's boot died on the resulting (correct) +/// "Cyclic __proto__ value" self-set rejection. Consult BOTH stores. This +/// never CREATES a namespace: if none was ever built, no user store can have +/// landed on one. +pub(crate) fn native_namespace_user_value(module: &str, prop: &str) -> Option { + if let Some(value) = native_namespace_prop_override_get(module, prop) { + return Some(value); + } + // Build the probe key BEFORE reading the cached namespace bits: the + // string allocation can run a moving collection, and the cache slot is + // rewritten by `scan_native_callable_export_roots_mut`, so bits read afterwards + // are current. + let key = crate::string::js_string_from_bytes(prop.as_ptr(), prop.len() as u32); + let ns_bits = NATIVE_MODULE_NAMESPACES.with(|cache| cache.borrow().get(module).copied())?; + let obj = (ns_bits & crate::value::POINTER_MASK) as *const ObjectHeader; + if obj.is_null() { + return None; + } + unsafe { super::field_get_set::native_module_own_field_by_key(obj, key) } + .map(|v| f64::from_bits(v.bits())) +} + fn bound_native_method_length(name: &str) -> Option { match name { "keepSocketAlive" => Some(1), @@ -922,7 +955,7 @@ unsafe fn native_module_property_by_name_impl( // codegen `NativeModuleRef` fast-path landed here without consulting the // side-table, so writes via `PutValueSet` didn't round-trip on reads. if consult_overrides { - if let Some(value) = native_namespace_prop_override_get(module_name, property_name) { + if let Some(value) = native_namespace_user_value(module_name, property_name) { return value; } } @@ -1078,6 +1111,14 @@ pub extern "C" fn js_native_module_esm_export_value(module: f64, property: f64) return f64::from_bits(crate::value::TAG_UNDEFINED); }; let module = normalize_native_module_alias(&module).to_string(); + // A user write to the member wins over the built-in snapshot below — + // this entry also serves property reads off the DEFAULT export object + // (`import fs from "node:fs"; fs.rename` after graceful-fs patched it), + // which is Node's live mutable CJS exports object. See + // `native_namespace_user_value`. + if let Some(value) = native_namespace_user_value(&module, &property) { + return value; + } let key = format!("{module}\0{property}"); if let Some(bits) = NATIVE_ESM_EXPORT_VALUES.with(|values| values.borrow().get(&key).copied()) { return f64::from_bits(bits); diff --git a/test-files/test_gap_9091_native_member_patch_roundtrip.ts b/test-files/test_gap_9091_native_member_patch_roundtrip.ts new file mode 100644 index 0000000000..40b16ff7cc --- /dev/null +++ b/test-files/test_gap_9091_native_member_patch_roundtrip.ts @@ -0,0 +1,34 @@ +// graceful-fs's polyfills run at module init in every esbuild-bundled CLI +// (pi, and anything else that pulls graceful-fs): +// +// var chdir = process.chdir; +// process.chdir = function (d) { ... }; +// if (Object.setPrototypeOf) Object.setPrototypeOf(process.chdir, chdir); +// +// A write to a builtin namespace member must win every subsequent read. +// Perry's name-keyed read entries handed back the canonical BOUND_METHOD +// closure regardless of the store, so the setPrototypeOf received the SAME +// closure twice and pi's boot died on the (correct) "Cyclic __proto__ value" +// self-set rejection. +import fs from "node:fs"; + +const chdir = process.chdir; +process.chdir = function (d: string) { chdir.call(process, d); }; +console.log("process.chdir patched:", process.chdir !== chdir); +if (Object.setPrototypeOf) Object.setPrototypeOf(process.chdir, chdir); +console.log("process.chdir proto:", Object.getPrototypeOf(process.chdir) === chdir); + +const fs$rename = fs.rename; +fs.rename = function rename(a: any, b: any, cb: any) { return fs$rename(a, b, cb); } as any; +console.log("fs.rename patched:", fs.rename !== fs$rename); +if (Object.setPrototypeOf) Object.setPrototypeOf(fs.rename, fs$rename); +console.log("fs.rename proto:", Object.getPrototypeOf(fs.rename) === fs$rename); + +const fs$read = fs.read; +fs.read = function read(fd: any, buffer: any, offset: any, length: any, position: any, cb: any) { + return (fs$read as any)(fd, buffer, offset, length, position, cb); +} as any; +console.log("fs.read patched:", fs.read !== fs$read); +if (Object.setPrototypeOf) Object.setPrototypeOf(fs.read, fs$read); +console.log("fs.read proto:", Object.getPrototypeOf(fs.read) === fs$read); +console.log("done"); From 649784f071b086fd016bf6ce14d9366b0e07c8c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 30 Aug 2026 07:02:26 +0200 Subject: [PATCH 3/3] style: rustfmt the closure-identity fix --- crates/perry-codegen/src/expr/closure.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/perry-codegen/src/expr/closure.rs b/crates/perry-codegen/src/expr/closure.rs index ce40282104..1fcfb52a90 100644 --- a/crates/perry-codegen/src/expr/closure.rs +++ b/crates/perry-codegen/src/expr/closure.rs @@ -304,8 +304,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { // per resume with the same per-activation box captures. User // arrows and function expressions always mint fresh objects. let is_plain_async_step = is_plain_async_step_body(body); - let singleton_identity_safe = - is_plain_async_step && (*is_arrow || captures_all_boxed); + let singleton_identity_safe = is_plain_async_step && (*is_arrow || captures_all_boxed); let no_capture_singleton = is_plain_async_step && *is_arrow && total_caps == 0; let captured_singleton = singleton_identity_safe && !no_capture_singleton && !writes_unboxed_capture;