fix(runtime): share symbol-keyed metadata by reference across wrappers of one native handle (#5715)

* fix(runtime): #5437 — share native-handle symbol meta across request wrappers (Next.js resolvedPathname)

A native handle (small-id NaN-boxed POINTER, e.g. a node:http IncomingMessage)
carries per-request metadata in the symbol side table keyed by its handle id.
Node shares ONE metadata object by reference across every NodeNextRequest
wrapper that re-news around the same IncomingMessage, so:

 - a wrapper read `req[NEXT_REQUEST_META]` resolves to the shared object, and
 - a write-back `this._req[SYM] = this[SYM]` is harmless when `this[SYM]` is
   the shared object.

In Perry the sharing broke in two ways for a late SSR-bundled wrapper:

 1. WIPE: the bundled wrapper reached the write-back with an *undefined*
    `this[SYM]`, clobbering the handle's existing metadata to undefined and
    losing `resolvedPathname` → Next's `resolvedPathname must be set` invariant.
 2. UNSEEDED READ: the bundled wrapper's own `[SYM]` was never set, so the
    render's `getRequestMeta` read undefined off it even though its `_req`
    handle held the shared meta.

Fix (both gated tightly to native handle-band receivers):
 - set_symbol_property: an `undefined` write onto a handle-band receiver that
   already holds a non-undefined entry is a no-op (never adds information; the
   by-reference object the handle still points at is what Node keeps).
 - js_object_get_symbol_property: on a heap-wrapper side-table miss, fall
   through to the wrapper's `_req` native handle's symbol meta.

Regression tests cover the share, the wipe-guard, and that a plain heap object
setting a symbol prop to undefined still clears it.

* test(symbol): suppress GC in handle-meta-share tests (parallel cargo-test flake)

The two #5437 handle-meta tests compared a heap `meta` pointer that the
GC-rooted SYMBOL_PROPERTIES side table rewrites on a move while the local
stays stale — deterministic serially (how it was validated), but under
parallel cargo-test a sibling allocation triggers a GC mid-test → spurious
`got != meta`. Wrap each body in gc_suppress()/gc_unsuppress() so the
objects can't move; unsuppress before the assert so a panic can't leak
suppression. The symbol-meta fix itself is unchanged.

* fix(runtime): #5437 — GC-invariant handle-meta tests + CodeRabbit fixes

(A) Make the handle-meta-share tests deterministic under parallel cargo-test
without relying on gc_suppress (which only gates gc_check_trigger, not the
1MB block-alloc trigger). Use an immovable NaN-boxed NUMBER for the metadata
value: numbers are never pointers, so the SYMBOL_PROPERTIES side-table
scanner never rewrites them on a GC move -> got.to_bits() == meta.to_bits()
holds across any collection. wrapper_reads_share_underlying_handle_meta also
forces a full gc() up front to compact the arena so its few allocations
can't trip a mid-test block-alloc GC. The metadata tests now use the exact
Symbol.for("NextInternalRequestMeta") symbol (required by the narrowed
guard below).

(B1) req_handle_symbol_fallback (get.rs): drop the is_small_handle pre-check.
is_valid_obj_ptr already rejects handles and accepts a valid heap object even
at a low address; the pre-check wrongly rejected a real low-address wrapper.

(B2) set_symbol_property (properties.rs): narrow the undefined-write no-op to
ONLY Symbol.for("NextInternalRequestMeta") on a handle-band receiver
(resolved+cached once). Any other handle symbol — including clearing it with
undefined — now stores normally. Added a regression test
(undefined_write_clears_non_metadata_symbol_on_handle).

* test(symbol): handle-meta test ids must be < 0x1000 (Linux is_valid_obj_ptr HEAP_MIN)

The handle-meta tests failed ONLY on x86_64-LINUX CI (pass on arm64 + x86_64
macOS — proven via Rosetta). Root: is_valid_obj_ptr uses HEAP_MIN=0x1000 on
Linux but 0x200_0000_0000 on macOS, so synthetic handle ids 0x4321/0x5678/0x6789
(all >= 0x1000) were classified as valid heap objects on Linux, breaking the
handle-band gate. On macOS they're far below HEAP_MIN so the gate worked. Fix:
ids < 0x1000 (matching real tiny native handles), rejected as non-objects on
every platform. Not a GC race (corrects the prior gc_suppress/number-meta theory).

---------

Co-authored-by: Ralph Küpper <ralph2@skelpo.com>

Author: proggeramlug

crates/perry-runtime/src/symbol/get.rs

@@ -61,6 +61,48 @@ pub(crate) unsafe fn own_symbol_property(obj_f64: f64, sym_f64: f64) -> Option<f
     None
 }
 
+/// #5437: resolve a symbol-keyed read against the underlying native handle a
+/// request wrapper aliases via its `_req` field. Returns `None` unless the
+/// receiver is a heap object whose `_req` is a small handle (POINTER-tagged,
+/// not a heap object) that holds the requested symbol in the side table.
+unsafe fn req_handle_symbol_fallback(obj_f64: f64, sym_f64: f64) -> Option<f64> {
+    let bits = obj_f64.to_bits();
+    if (bits >> 48) != 0x7FFD {
+        return None;
+    }
+    let raw = (bits & POINTER_MASK) as usize;
+    // Only heap-object wrappers carry a `_req` field; skip handle receivers.
+    // `is_valid_obj_ptr` already rejects small native handles (it validates the
+    // GcHeader) and accepts a genuine heap object even at a low address, so the
+    // extra `is_small_handle` pre-check would have wrongly rejected a real heap
+    // wrapper that happens to live in the low band.
+    if !crate::object::is_valid_obj_ptr(raw as *const u8) {
+        return None;
+    }
+    let key = b"_req";
+    let kh = crate::string::js_string_from_bytes(key.as_ptr(), key.len() as u32);
+    let req = crate::object::js_object_get_field_by_name_f64(
+        raw as *const crate::object::ObjectHeader,
+        kh as *const crate::StringHeader,
+    );
+    let rbits = req.to_bits();
+    if (rbits >> 48) != 0x7FFD {
+        return None;
+    }
+    let rraw = (rbits & POINTER_MASK) as usize;
+    // The `_req` must be a small native handle, never another heap object — a
+    // heap `_req` would be served by its own normal symbol path, and recursing
+    // into it risks loops.
+    if !crate::value::addr_class::is_small_handle(rraw)
+        || crate::object::is_valid_obj_ptr(rraw as *const u8)
+    {
+        return None;
+    }
+    // Read only what the handle actually holds in the side table (no deref of
+    // the handle id as a heap object).
+    own_symbol_property(req, sym_f64)
+}
+
 unsafe fn object_header_ptr_from_value_bits(bits: u64) -> Option<usize> {
     let top16 = bits >> 48;
     let raw = if top16 == 0x7FFD {
@@ -371,6 +413,21 @@ pub unsafe extern "C" fn js_object_get_symbol_property(obj_f64: f64, sym_f64: f6
     if let Some(v) = own_symbol_property(obj_f64, sym_f64) {
         return v;
     }
+    // #5437 (Next.js): a heap request *wrapper* whose own symbol entry misses
+    // may be one of several `NodeNextRequest`s wrapping the SAME underlying
+    // native IncomingMessage handle (stored in its `_req` field). Node shares
+    // one per-request metadata object by reference across every such wrapper —
+    // the wrapper's ctor does `this[SYM] = this._req[SYM] || {}`. When that
+    // share didn't land on this particular wrapper (a late SSR-bundled copy
+    // never had its `[SYM]` seeded), fall through to the underlying handle's
+    // symbol meta so the read still observes the shared object, matching Node.
+    // Gated tightly: only fires on a side-table MISS, only when `_req` resolves
+    // to a small native handle (POINTER-tagged, below HANDLE_BAND_MAX, not a
+    // real heap object), and only returns a value the handle actually holds —
+    // so ordinary objects (no `_req`, or a heap `_req`) are unaffected.
+    if let Some(v) = req_handle_symbol_fallback(obj_f64, sym_f64) {
+        return v;
+    }
     let sym_key = sym_key_from_f64(sym_f64);
     if sym_key != 0 {
         let jsval = crate::value::JSValue::from_bits(bits);
@@ -696,3 +753,170 @@ fn class_chain_has_method(class_id: u32, name: &str) -> bool {
     }
     false
 }
+
+#[cfg(test)]
+mod handle_meta_share_tests {
+    //! #5437: a native handle's symbol-keyed metadata must be shared by
+    //! reference across heap wrappers that alias it (Next.js NodeNextRequest
+    //! over an IncomingMessage), and must survive an `undefined` write-back.
+    use super::*;
+
+    const POINTER_TAG_BITS: u64 = 0x7FFD_0000_0000_0000;
+
+    // A registered `Symbol.for(key)` as a NaN-boxed f64.
+    unsafe fn registered_symbol(key: &str) -> f64 {
+        let kh = js_string_from_bytes(key.as_ptr(), key.len() as u32);
+        let key_f64 = crate::value::js_nanbox_string(kh as i64);
+        super::constructors::js_symbol_for(key_f64)
+    }
+
+    // The exact registered symbol Next.js uses; the undefined-write wipe guard
+    // is gated to THIS symbol, so the metadata tests must use it (not a
+    // test-suffixed variant) for the no-op behaviour to fire.
+    unsafe fn next_request_meta_symbol() -> f64 {
+        registered_symbol("NextInternalRequestMeta")
+    }
+
+    // A small native handle id NaN-boxed as POINTER (e.g. an IncomingMessage).
+    // MUST stay below 0x1000: `is_valid_obj_ptr` uses HEAP_MIN=0x1000 on Linux
+    // (0x200_0000_0000 on macOS), so an id >= 0x1000 is misclassified as a valid
+    // heap object ON LINUX — which breaks the handle-band gate and fails these
+    // tests in CI (they pass on macOS, where the id is far below HEAP_MIN). Real
+    // native handles are tiny, so a sub-0x1000 id faithfully models them.
+    fn handle_value(id: u64) -> f64 {
+        f64::from_bits(POINTER_TAG_BITS | id)
+    }
+
+    // An IMMOVABLE metadata value: a plain NaN-boxed number. Unlike a heap
+    // object, a number is never a pointer, so the SYMBOL_PROPERTIES side-table
+    // scanner never rewrites it on a GC move — its bits are invariant across any
+    // collection. The #5437 fix logic (no-op on undefined, `_req` fallthrough)
+    // is value-agnostic, so a number tests it faithfully without depending on
+    // GC suppression to keep a heap `meta` from moving.
+    fn immovable_meta() -> f64 {
+        // A normal finite double whose bits are stable and unambiguous.
+        1234.5_f64
+    }
+
+    #[test]
+    fn wrapper_reads_share_underlying_handle_meta() {
+        unsafe {
+            // Compact the arena up front with a full GC so the few small
+            // allocations this test makes can't trip a mid-test block-alloc GC
+            // (which bypasses `gc_suppress`). `gc_suppress` is kept as belt-and-
+            // braces; the immovable number `meta` makes the assertion itself
+            // GC-invariant regardless.
+            crate::gc::js_gc_collect();
+            crate::gc::gc_suppress();
+            let sym = next_request_meta_symbol();
+            // Pick a handle id well inside the small-handle band but unlikely to
+            // collide with another test's side-table entry.
+            let handle = handle_value(0x321);
+
+            // The per-request metadata value lives on the handle. Use an
+            // immovable number so a GC can't invalidate the comparison.
+            let meta = immovable_meta();
+            super::properties::js_object_set_symbol_property(handle, sym, meta);
+
+            // A heap wrapper that aliases the handle via `_req` but never had
+            // its own `[sym]` seeded.
+            let wrapper_obj = crate::object::js_object_alloc(0, 1);
+            assert!(!wrapper_obj.is_null());
+            let req_key = js_string_from_bytes(b"_req".as_ptr(), 4);
+            crate::object::js_object_set_field_by_name(wrapper_obj, req_key, handle);
+            let wrapper = crate::value::js_nanbox_pointer(wrapper_obj as i64);
+
+            // Reading the symbol off the wrapper falls through to the handle's
+            // shared meta — the exact Node-by-reference semantics.
+            let got = js_object_get_symbol_property(wrapper, sym);
+            // Unsuppress before asserting so a panic can't leave GC suppressed
+            // for sibling tests on this thread.
+            crate::gc::gc_unsuppress();
+            assert_eq!(
+                got.to_bits(),
+                meta.to_bits(),
+                "wrapper symbol read should share the handle's meta value"
+            );
+        }
+    }
+
+    #[test]
+    fn undefined_write_does_not_clobber_handle_meta() {
+        unsafe {
+            // Immovable number `meta` → no heap object to move → assertion is
+            // GC-invariant; `gc_suppress` is defensive only.
+            crate::gc::gc_suppress();
+            let sym = next_request_meta_symbol();
+            let handle = handle_value(0x654);
+            let meta = immovable_meta();
+            super::properties::js_object_set_symbol_property(handle, sym, meta);
+
+            // The `this._req[SYM] = this[SYM]` write-back where `this[SYM]` is
+            // undefined must NOT erase the handle's existing meta.
+            let undef = f64::from_bits(TAG_UNDEFINED);
+            super::properties::js_object_set_symbol_property(handle, sym, undef);
+
+            let got = js_object_get_symbol_property(handle, sym);
+            crate::gc::gc_unsuppress();
+            assert_eq!(
+                got.to_bits(),
+                meta.to_bits(),
+                "an undefined write must not clobber a handle's existing meta"
+            );
+        }
+    }
+
+    #[test]
+    fn undefined_write_to_plain_heap_object_still_clears() {
+        unsafe {
+            // The wipe-guard is gated to handle-band receivers; a normal heap
+            // object setting a symbol prop to undefined must still take effect.
+            let sym = registered_symbol("plainObjSym@@test_clear");
+            let obj_ptr = crate::object::js_object_alloc(0, 0);
+            let obj = crate::value::js_nanbox_pointer(obj_ptr as i64);
+            let v = immovable_meta();
+            super::properties::js_object_set_symbol_property(obj, sym, v);
+            let undef = f64::from_bits(TAG_UNDEFINED);
+            super::properties::js_object_set_symbol_property(obj, sym, undef);
+            let got = js_object_get_symbol_property(obj, sym);
+            assert_eq!(
+                got.to_bits(),
+                TAG_UNDEFINED,
+                "heap-object symbol prop set to undefined must read undefined"
+            );
+        }
+    }
+
+    #[test]
+    fn undefined_write_clears_non_metadata_symbol_on_handle() {
+        unsafe {
+            // The wipe-guard is narrowed to `Symbol.for("NextInternalRequestMeta")`.
+            // Any OTHER symbol on a handle must clear normally with `undefined`.
+            crate::gc::gc_suppress();
+            let sym = registered_symbol("someOtherHandleSym@@test_clear");
+            // Distinct handle id so this doesn't alias the metadata tests.
+            let handle = handle_value(0x789);
+            let v = immovable_meta();
+            super::properties::js_object_set_symbol_property(handle, sym, v);
+
+            // Sanity: the value is observable before the clear.
+            let before = js_object_get_symbol_property(handle, sym);
+            assert_eq!(
+                before.to_bits(),
+                v.to_bits(),
+                "non-metadata handle symbol should be set before clearing"
+            );
+
+            // Writing undefined to a NON-metadata symbol on a handle MUST clear it.
+            let undef = f64::from_bits(TAG_UNDEFINED);
+            super::properties::js_object_set_symbol_property(handle, sym, undef);
+            let got = js_object_get_symbol_property(handle, sym);
+            crate::gc::gc_unsuppress();
+            assert_eq!(
+                got.to_bits(),
+                TAG_UNDEFINED,
+                "a non-metadata symbol on a handle must be clearable with undefined"
+            );
+        }
+    }
+}

crates/perry-runtime/src/symbol/properties.rs

@@ -216,6 +216,34 @@ unsafe fn infer_symbol_function_name(sym_key: usize, val_bits: u64) {
     register_closure_name_if_absent(val_bits, &inferred);
 }
 
+/// Resolve (and cache) the registered `Symbol.for("NextInternalRequestMeta")`
+/// pointer used by Next.js to stash per-request metadata on the underlying
+/// IncomingMessage handle. Returns the symbol's stable `sym_key` (its leaked
+/// `SymbolHeader*`), or 0 if it can't be resolved. The undefined-write wipe
+/// guard below is narrowed to THIS symbol so ordinary handle symbol writes —
+/// including clearing a non-metadata symbol with `undefined` — behave normally.
+fn next_request_meta_sym_key() -> usize {
+    use std::sync::atomic::{AtomicUsize, Ordering};
+    // 0 = "not resolved yet", usize::MAX would be a sentinel we never need
+    // because the registered symbol pointer is always a real, non-zero address.
+    static CACHED: AtomicUsize = AtomicUsize::new(0);
+    let cached = CACHED.load(Ordering::Relaxed);
+    if cached != 0 {
+        return cached;
+    }
+    const KEY: &[u8] = b"NextInternalRequestMeta";
+    let sym_key = unsafe {
+        let kh = js_string_from_bytes(KEY.as_ptr(), KEY.len() as u32);
+        let key_f64 = crate::value::js_nanbox_string(kh as i64);
+        let sym = super::constructors::js_symbol_for(key_f64);
+        sym_key_from_f64(sym)
+    };
+    if sym_key != 0 {
+        CACHED.store(sym_key, Ordering::Relaxed);
+    }
+    sym_key
+}
+
 unsafe fn set_symbol_property(obj_f64: f64, sym_f64: f64, value_f64: f64) -> f64 {
     if let Some(acc) = accessors::symbol_accessor_property(obj_f64, sym_f64) {
         if acc.set != 0 {
@@ -232,6 +260,46 @@ unsafe fn set_symbol_property(obj_f64: f64, sym_f64: f64, value_f64: f64) -> f64
     if obj_key == 0 || sym_key == 0 {
         return value_f64;
     }
+    // #5437 (Next.js): a native HANDLE (small-id NaN-boxed POINTER, e.g. the
+    // node:http IncomingMessage) carries per-request metadata in the symbol
+    // side table keyed by its handle id. Node shares one metadata object by
+    // reference across every wrapper that re-`new`s around the same
+    // IncomingMessage, so a wrapper write-back like
+    // `this._req[NEXT_REQUEST_META] = this[NEXT_REQUEST_META]` is harmless when
+    // `this[...]` is the shared object. In Perry a late-bundled wrapper can
+    // reach that write-back with an *undefined* `this[...]`, which would CLOBBER
+    // the handle's existing (non-undefined) metadata — wiping
+    // `resolvedPathname` and tripping Next's `resolvedPathname must be set`
+    // invariant. Treat an `undefined` write onto a handle-band receiver that
+    // already holds a non-undefined entry as a no-op: it never *adds*
+    // information, and the by-reference object the handle still points at is
+    // exactly what Node would keep.
+    //
+    // Gated narrowly so it only protects the Next request-metadata flow:
+    //   (1) the symbol is `Symbol.for("NextInternalRequestMeta")`,
+    //   (2) the receiver is a handle-band value (id below HANDLE_BAND_MAX and
+    //       not a real heap object),
+    //   (3) the write value is `undefined`, and
+    //   (4) a non-undefined entry already exists.
+    // Any OTHER symbol on a handle — including legitimately clearing it by
+    // writing `undefined` — falls through to the normal store path below.
+    {
+        let raw = (obj_f64.to_bits() & crate::value::POINTER_MASK) as usize;
+        let meta_key = next_request_meta_sym_key();
+        if meta_key != 0
+            && sym_key == meta_key
+            && (obj_f64.to_bits() >> 48) == 0x7FFD
+            && crate::value::addr_class::is_small_handle(raw)
+            && !crate::object::is_valid_obj_ptr(raw as *const u8)
+            && value_f64.to_bits() == TAG_UNDEFINED
+        {
+            if let Some(existing) = symbol_property_root_bits(obj_key, sym_key) {
+                if existing != TAG_UNDEFINED {
+                    return value_f64;
+                }
+            }
+        }
+    }
     // `Array.prototype[Symbol.iterator] = fn` disables the array fast path in
     // `js_get_iterator` so destructuring / GetIterator see the patched method.
     crate::array::note_array_proto_iterator_write(obj_key, sym_key);