fix(codegen,runtime): closure-literal singletons broke function identity — pi boots

crates/perry-codegen/src/expr/closure.rs

@@ -266,13 +266,12 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
             // 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,29 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
                 && 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 +403,6 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
             // 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))

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!(

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<f64> {
+    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<u32> {
     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);

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);

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");