fix(gc): rekey built-in closure metadata after moves

changelog.d/8397-builtin-closure-gc-metadata.md

@@ -0,0 +1 @@
+Keep per-instance built-in closure metadata synchronized when copying GC relocates a closure. Value-called prototype methods such as `Array.prototype.valueOf` now keep dispatching after a minor collection, so relational comparisons involving arrays no longer switch from correct string comparison to a permanent `NaN` result.

crates/perry-runtime/src/gc/dead_owner.rs

@@ -353,6 +353,11 @@ pub(super) const DEAD_KEY_PRUNES: &[DeadKeyPrune] = &[
         owner: DeadKeyOwner::Closure,
         prune: crate::closure::prune_dead_closure_side_table_owners,
     },
+    DeadKeyPrune {
+        table: "BUILTIN_CLOSURE_LENGTH + BUILTIN_CLOSURE_NON_CONSTRUCTABLE",
+        owner: DeadKeyOwner::Closure,
+        prune: crate::object::prune_dead_builtin_closure_metadata_owners,
+    },
     // #8040: `FUNCTION_CLASS_IDS` is keyed by a synthetic-class function
     // value's closure address, and is REKEYED (not re-derived) when that
     // closure moves — so a dead key does not merely leak, the rekey walk

crates/perry-runtime/src/gc/mod.rs

@@ -992,6 +992,11 @@ pub fn gc_init() {
     // captured young values or future cache hits miss on stale addresses.
     reg_scanner!(crate::closure::scan_singleton_closure_roots_mut);
     reg_scanner!(crate::closure::scan_closure_dynamic_props_roots_mut);
+    // #8393: built-in prototype methods carry per-closure identity metadata
+    // keyed by their raw heap address. Copying minor GC moves those closures;
+    // keep the weak metadata keys aligned with the forwarded addresses so
+    // value-called methods still reach the prototype dispatch tower.
+    reg_scanner!(crate::object::scan_builtin_closure_metadata_roots_mut);
     reg_scanner!(crate::buffer::scan_buffer_own_props_roots_mut);
     // Generic per-handle expando properties (`blob.colors = [...]` and other
     // arbitrary own props on native HANDLE values). Keys are stable small handle

crates/perry-runtime/src/gc/tests/runtime_roots/side_table_scanners.rs

@@ -539,3 +539,63 @@ fn test_runtime_root_visitor_rewrites_metadata_without_marking() {
     RuntimeRootVisitor::for_rewrite(&valid_ptrs).visit_metadata_usize_slot(&mut metadata);
     assert_eq!(metadata, old_user as usize);
 }
+
+#[test]
+fn test_builtin_closure_metadata_follows_forwarded_owner() {
+    let _guard = GcTestIsolationGuard::new();
+    let _trigger_guard = GcTriggerThresholdTestGuard::suppress_automatic_triggers();
+    clear_marks();
+    clear_mark_seeds();
+
+    let nursery_owner = crate::arena::arena_alloc_gc(
+        std::mem::size_of::<crate::closure::ClosureHeader>(),
+        std::mem::align_of::<crate::closure::ClosureHeader>(),
+        GC_TYPE_CLOSURE,
+    ) as usize;
+    let valid_ptrs = build_valid_pointer_set();
+    let relocated_owner = crate::arena::arena_alloc_gc_old(
+        std::mem::size_of::<crate::closure::ClosureHeader>(),
+        std::mem::align_of::<crate::closure::ClosureHeader>(),
+        GC_TYPE_CLOSURE,
+    ) as usize;
+
+    crate::object::set_builtin_closure_length(nursery_owner, 3);
+    crate::object::set_builtin_closure_non_constructable(nursery_owner);
+
+    // These tables classify closures owned by the heap graph; their keys must
+    // not turn into an independent root during a mark phase.
+    crate::object::scan_builtin_closure_metadata_roots_mut(&mut RuntimeRootVisitor::for_mark(
+        &valid_ptrs,
+    ));
+    assert_unmarked_user_ptr(nursery_owner, "built-in closure metadata owner");
+
+    unsafe {
+        set_forwarding_address(
+            header_from_user_ptr(nursery_owner as *const u8) as *mut GcHeader,
+            relocated_owner as *mut u8,
+        );
+    }
+    crate::object::scan_builtin_closure_metadata_roots_mut(&mut RuntimeRootVisitor::for_rewrite(
+        &valid_ptrs,
+    ));
+
+    assert_eq!(crate::object::builtin_closure_length(nursery_owner), None);
+    assert_eq!(
+        crate::object::builtin_closure_length(relocated_owner),
+        Some(3)
+    );
+    assert!(!crate::object::builtin_closure_is_non_constructable(
+        nursery_owner
+    ));
+    assert!(crate::object::builtin_closure_is_non_constructable(
+        relocated_owner
+    ));
+
+    crate::object::prune_dead_builtin_closure_metadata_owners(&|owner| owner == relocated_owner);
+    assert_eq!(crate::object::builtin_closure_length(relocated_owner), None);
+    assert!(!crate::object::builtin_closure_is_non_constructable(
+        relocated_owner
+    ));
+    clear_marks();
+    clear_mark_seeds();
+}

crates/perry-runtime/src/object/native_module.rs

@@ -26,6 +26,8 @@ mod namespace_builders;
 mod web_locks;
 
 pub(crate) use callable_export_check::is_native_module_callable_export;
+#[cfg(test)]
+pub(crate) use callable_exports::builtin_closure_is_non_constructable;
 pub(crate) use callable_exports::{
     bound_native_callable_export_value, bound_native_callable_module_and_method,
     bound_native_callable_value_arity, buffer_constructor_value,
@@ -34,11 +36,13 @@ pub(crate) use callable_exports::{
     is_buffer_constructor_value, is_cluster_emitter_method, module_builtin_modules_value,
     module_cjs_cache_value, module_cjs_extensions_value, module_cjs_global_paths_value,
     module_cjs_path_cache_value, module_cjs_prototype_for_instance, module_constants_value,
-    native_string_value, scan_tls_derived_prototype_roots_mut, set_bound_native_closure_name,
-    set_builtin_closure_length, set_builtin_closure_non_constructable,
-    sqlite_session_constructor_value, sqlite_statement_sync_constructor_value,
-    timers_promises_parent_namespace, tls_constructor_prototype_is_instance_of,
-    util_inspect_default_options_value, zlib_codes_object,
+    native_string_value, prune_dead_builtin_closure_metadata_owners,
+    scan_builtin_closure_metadata_roots_mut, scan_tls_derived_prototype_roots_mut,
+    set_bound_native_closure_name, set_builtin_closure_length,
+    set_builtin_closure_non_constructable, sqlite_session_constructor_value,
+    sqlite_statement_sync_constructor_value, timers_promises_parent_namespace,
+    tls_constructor_prototype_is_instance_of, util_inspect_default_options_value,
+    zlib_codes_object,
 };
 pub(crate) use constants::get_native_module_constant;
 pub(crate) use module_keys::{native_module_enumerable_keys, native_module_has_enumerable_key};

crates/perry-runtime/src/object/native_module/callable_exports.rs

@@ -1425,6 +1425,61 @@ pub(crate) fn builtin_closure_is_non_constructable(closure: usize) -> bool {
     BUILTIN_CLOSURE_NON_CONSTRUCTABLE.with(|m| m.borrow().contains(&closure))
 }
 
+/// Rekey per-instance built-in closure metadata after a moving collection.
+///
+/// The keys are identities, not roots: prototype/global objects keep live
+/// built-in closures reachable, while dead closures must remain collectable.
+/// `visit_metadata_usize_slot` therefore only follows forwarding records.
+pub(crate) fn scan_builtin_closure_metadata_roots_mut(
+    visitor: &mut crate::gc::RuntimeRootVisitor<'_>,
+) {
+    BUILTIN_CLOSURE_LENGTH.with(|lengths| {
+        let mut lengths = lengths.borrow_mut();
+        let mut moved = Vec::new();
+        for old_owner in lengths.keys().copied() {
+            let mut new_owner = old_owner;
+            if visitor.visit_metadata_usize_slot(&mut new_owner) && new_owner != old_owner {
+                moved.push((old_owner, new_owner));
+            }
+        }
+        for (old_owner, new_owner) in moved {
+            if let Some(length) = lengths.remove(&old_owner) {
+                lengths.insert(new_owner, length);
+            }
+        }
+    });
+
+    BUILTIN_CLOSURE_NON_CONSTRUCTABLE.with(|non_constructable| {
+        let mut non_constructable = non_constructable.borrow_mut();
+        let mut moved = Vec::new();
+        for old_owner in non_constructable.iter().copied() {
+            let mut new_owner = old_owner;
+            if visitor.visit_metadata_usize_slot(&mut new_owner) && new_owner != old_owner {
+                moved.push((old_owner, new_owner));
+            }
+        }
+        for (old_owner, new_owner) in moved {
+            non_constructable.remove(&old_owner);
+            non_constructable.insert(new_owner);
+        }
+    });
+}
+
+/// Drop metadata for closures proved dead by the collector before their arena
+/// addresses can be recycled for unrelated objects.
+pub(crate) fn prune_dead_builtin_closure_metadata_owners(is_dead_owner: &dyn Fn(usize) -> bool) {
+    BUILTIN_CLOSURE_LENGTH.with(|lengths| {
+        lengths
+            .borrow_mut()
+            .retain(|owner, _| !is_dead_owner(*owner));
+    });
+    BUILTIN_CLOSURE_NON_CONSTRUCTABLE.with(|non_constructable| {
+        non_constructable
+            .borrow_mut()
+            .retain(|owner| !is_dead_owner(*owner));
+    });
+}
+
 pub(crate) fn builtin_closure_is_non_constructable_value(value: f64) -> bool {
     let jv = JSValue::from_bits(value.to_bits());
     if !jv.is_pointer() {

scripts/gc_rekeyed_key_tables.json

@@ -97,6 +97,12 @@
       "death": "dead_owner:prune_dead_transition_cache_entries",
       "why": "#8192: registered prune drops the whole cache entry when either weak half (prev_keys keys-array, key_ptr interned string) is dead; next_keys is a strong root and cannot be."
     },
+    {
+      "site": "crates/perry-runtime/src/object/native_module/callable_exports.rs::scan_builtin_closure_metadata_roots_mut",
+      "table": "BUILTIN_CLOSURE_LENGTH / BUILTIN_CLOSURE_NON_CONSTRUCTABLE",
+      "death": "dead_owner:prune_dead_builtin_closure_metadata_owners",
+      "why": "#8393: both tables are retained on the GC_TYPE_CLOSURE-narrowed predicate over their closure-address keys."
+    },
     {
       "site": "crates/perry-runtime/src/object/shapes.rs::scan_shape_table_rekey_mut",
       "table": "state().shapes.inner (descriptors + indices)",