diff --git a/crates/perry-codegen/src/expr/closure.rs b/crates/perry-codegen/src/expr/closure.rs index 9a22c66624..1fcfb52a90 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,29 @@ 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 +403,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/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_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); 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");