fix(lru-cache): faithful JS-value keys/values, GC rooting, TTL + updateAgeOnGet

changelog.d/7136-lru-cache-faithful.md

@@ -0,0 +1,120 @@
+### Fixed
+
+- **`lru-cache` native binding (`perry-ext-lru-cache`) is now faithful to the
+  npm `lru-cache` API for real-world usage.** The previous wrapper only handled
+  numeric (`f64`) keys and values with no TTL, so a cache keyed on strings with
+  object/string values (e.g. a typical caller's `new LRUCache({ max, ttl,
+  updateAgeOnGet })`) silently misbehaved. Two defects are fixed:
+
+  - **Keys and values are treated as real JS values, not raw `f64` bit
+    patterns.** String keys now hash and compare by **content** (the NaN-boxed
+    `StringHeader` is materialized via `js_get_string_pointer_unified` and keyed
+    on its bytes), so a `get("k")` after `set("k", …)` hits even when the two
+    `"k"` strings are distinct allocations or an SSO short string vs a heap
+    string. Number/boolean/null/undefined keys key by canonical value
+    (SameValueZero: `+0`/`-0` and all `NaN`s unified).
+  - **Cached heap values are GC roots for as long as they are cached.** A
+    mutable root scanner (`gc_register_mutable_root_scanner_named`) visits every
+    cached value slot each GC cycle, so stored objects/strings are marked and
+    rewritten to their forwarded address under copying evacuation — fixing a
+    use-after-free where a cached value was collected out from under the cache
+    (the "value is not a function" class of bug).
+
+- **Constructor honors the options object.** `new LRUCache({ max, ttl,
+  updateAgeOnGet })` is parsed by the runtime from the NaN-boxed options object
+  (codegen now forwards the whole object instead of statically extracting only
+  `max`, so dynamic/variable options work). `ttl` gives per-entry expiry on the
+  `performance.now()` clock (`get`/`has`/`peek` treat an expired entry as
+  absent; `get` evicts it); `updateAgeOnGet` resets an entry's TTL clock on a
+  live `get`. `peek` is now wired into method dispatch.
+
+- **Constructor options are validated exactly as npm validates them, so a
+  bad `max` throws instead of aborting the process.** `option_number`
+  accepted any finite number and `n as usize` saturated, handing the backing
+  map a capacity request it could not satisfy: `new LRUCache({ max: 1e12 })`
+  reserved a 10^12-bucket table and killed the process with no JS-visible
+  error. Rather than invent a bound, every case was measured against
+  `lru-cache@11.5.2` on the pinned oracle (Node 26.5.1) and reproduced
+  message for message:
+
+  | `new LRUCache(…)` | throws |
+  |---|---|
+  | `()` | `TypeError: Cannot read properties of undefined (reading 'max')` |
+  | `(null)` | `TypeError: Cannot read properties of null (reading 'max')` |
+  | `(5)`, `("x")`, `({})`, `({ max: 0 })`, `({ max: -0 })` | `TypeError: At least one of max, maxSize, or ttl is required` |
+  | `({ max: -1 \| 1.5 \| Infinity \| NaN \| "3" \| true \| null })` | `TypeError: max option must be a nonnegative integer` |
+  | `({ max: 2**32 })` … `({ max: MAX_SAFE_INTEGER })` | `RangeError: Invalid array length` |
+  | `({ max: 2**53 })`, `({ max: 1e300 })` | `Error: invalid max value: <n>` |
+  | `({ max: 3, ttl: -5 \| 1.5 \| Infinity \| "5" })` | `TypeError: ttl must be a positive integer if specified` |
+
+  The two upper bounds are npm's own: it builds its index arrays with
+  `Array.from({ length: max })` (past the JS array-length limit that is a
+  `RangeError`) after a `getUintArray(max)` lookup that returns `null` past
+  `Number.MAX_SAFE_INTEGER` (a plain `Error`). **This is a behavior change
+  for `new LRUCache()` and `new LRUCache(100)`**, which previously fell
+  through to a silent `max=100`; npm throws for both, and so does Perry now.
+  `max: 0` together with a `ttl` is npm's legal unbounded cache and is
+  supported.
+
+  Where Perry deliberately differs: the backing map grows lazily instead of
+  reserving `max` buckets up front, so a large-but-legal `max` (`1e8`, say)
+  constructs instantly here where npm OOMs Node. Nothing observes that
+  except by not running out of memory.
+
+  Not yet implemented (unchanged ABI carries only `(key, value)`):
+  `maxSize`/`sizeCalculation`, `dispose`/`disposeAfter`, `fetch`, `allowStale`,
+  per-call option objects, and the iterator surface. Because `maxSize` is
+  unimplemented it also does not satisfy npm's "at least one of max, maxSize,
+  or ttl" requirement — a `maxSize`-only cache constructs on npm but throws
+  here, which fails loudly instead of yielding a silently unbounded cache.
+  npm's `UnboundedCacheWarning` for `ttl`-only caches is not emitted. Object-
+  identity keys are supported by pointer identity but are not tracked across a
+  GC relocation; primitive keys are the GC-safe path.
+
+  Two further divergences, both pre-existing and both found by diffing a
+  compiled probe against the npm package under Node 26.5.1 (41 of 45 output
+  lines are byte-identical, including every option-validation case above):
+
+  - `cache.size` is wired as a *method* row, so `cache.size()` works but
+    npm's `cache.size` property read yields `undefined`. Unchanged here.
+  - npm caches its TTL clock and only refreshes it from a `setTimeout`
+    (its `ttlResolution`), so code that blocks the event loop sees entries
+    stay live indefinitely on npm. This binding reads `performance.now()`
+    on every access, so a blocking loop does observe expiry.
+
+  Constructor options are read through the runtime's boxed-receiver getter
+  (`js_object_get_field_by_name_boxed`) rather than being unboxed here to a
+  `*const ObjectHeader`. `options` is an untrusted runtime value — an
+  array, a function and a native handle id are all pointer-tagged — and the
+  unboxed getter dereferences its argument on faith. This also deletes a
+  hand-rolled `>= 0x1000` band literal, the kind of open-coded address test
+  the addr-class ratchet exists to prevent. Behavior is unchanged and now
+  pinned by a test: strings, empty and populated arrays, and handle-band
+  ids all yield npm's "At least one of max, maxSize, or ttl is required",
+  which is what npm gives for any heap value lacking a `max` own property.
+
+  The GC-survival test moved to its own test binary
+  (`crates/perry-ext-lru-cache/tests/gc_survival.rs`) and now asserts that
+  the collector *relocated* the cached value, not merely that it is still
+  readable — a non-moving collection satisfies the latter without
+  exercising one line of the scanner's forwarding-pointer rewrite
+  (#6942/#6946). It needs its own process to do that: the collector
+  conservatively pins any nursery object a stack word points at, so after
+  even one other test in the same binary the minor reports
+  `copied_objects=0` instead of `1`. Verified in both directions —
+  `PERRY_GEN_GC=0` (non-moving mark-sweep) trips the new assertion. The
+  address is carried across the collection XOR-folded, from an
+  `#[inline(never)]` helper, so the test cannot conservatively pin the very
+  string whose relocation it asserts.
+
+  Both test files build their options objects null-prototype. Building them
+  the ordinary way (`js_object_alloc` + `js_object_set_field_by_name`)
+  destabilizes the collector for the rest of the process: SIGSEGV in
+  `gc::copying::scan_slot` under `--test-threads=1`, and 1 failure in 12
+  runs otherwise, landing in whichever unrelated test runs next. Null-proto
+  objects measured 0 in 40. Only own properties are read, so the prototype
+  is immaterial to what is under test, and the compiled A/B covers the
+  ordinary literal a real caller writes. The underlying fault is a runtime
+  bug, reported separately rather than worked around silently.
+
+  Tracking: #466 (Phase 5 native bindings). PR #7136.

crates/perry-codegen/src/lower_call/builtin.rs

@@ -521,37 +521,27 @@ pub(super) fn lower_builtin_new(
             let result = ctx.block().call(DOUBLE, runtime_fn, &[(DOUBLE, &opts_box)]);
             Ok(Some(result))
         }
-        // lru-cache LRUCache — `new LRUCache({ max: N })`. Runtime takes
-        // a single `max: f64`. Extract the `max` field from the options
-        // literal (handles both raw `Expr::Object(props)` and Phase 3's
-        // `Expr::New { __AnonShape_N }` shape via `extract_options_fields`);
-        // default to 100 when no options literal is detected (matches the
-        // npm `lru-cache` library's behavior for `new LRUCache()` with
-        // missing max — it warns + falls back, we just fall back).
+        // lru-cache LRUCache — `new LRUCache({ max, ttl, updateAgeOnGet })`.
+        // The runtime parses the whole NaN-boxed options object itself
+        // (`js_lru_cache_new(options: f64)`), so we just lower the options
+        // argument and hand it through — no static field extraction, which
+        // means dynamic/variable options objects work too. A missing options
+        // argument passes `undefined`, which the runtime rejects with the
+        // same `TypeError` npm's constructor destructuring raises.
         "LRUCache" => {
-            let max_val = if let Some(opts_arg) = args.first() {
-                let mut found_max: Option<String> = None;
-                if let Some(props) = extract_options_fields(ctx, opts_arg) {
-                    for (k, vexpr) in &props {
-                        if k == "max" {
-                            found_max = Some(lower_expr(ctx, vexpr)?);
-                        } else {
-                            // Lower other fields for side effects (e.g. ttl
-                            // option's setter calls).
-                            let _ = lower_expr(ctx, vexpr)?;
-                        }
-                    }
-                } else {
-                    // Non-literal arg (variable, dynamic shape) — lower for
-                    // side effects only; cannot extract max statically.
-                    let _ = lower_expr(ctx, opts_arg)?;
-                }
-                found_max.unwrap_or_else(|| "100.0".to_string())
+            let opts_val = if let Some(opts_arg) = args.first() {
+                lower_expr(ctx, opts_arg)?
             } else {
-                "100.0".to_string()
+                double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED))
             };
+            // npm's constructor ignores everything past the options object,
+            // but the arguments are still evaluated — lower the tail for its
+            // side effects so `new LRUCache(opts, f())` still calls `f`.
+            for arg in args.iter().skip(1) {
+                let _ = lower_expr(ctx, arg)?;
+            }
             let blk = ctx.block();
-            let handle = blk.call(I64, "js_lru_cache_new", &[(DOUBLE, &max_val)]);
+            let handle = blk.call(I64, "js_lru_cache_new", &[(DOUBLE, &opts_val)]);
             Ok(Some(nanbox_pointer_inline(blk, &handle)))
         }
         // (`WebSocketServer` is handled by an earlier branch lower in this

crates/perry-codegen/src/lower_call/native_table/node_misc.rs

@@ -311,6 +311,15 @@ pub(super) const NODE_MISC_ROWS: &[NativeModSig] = &[
         args: &[],
         ret: NR_F64,
     },
+    NativeModSig {
+        module: "lru-cache",
+        has_receiver: true,
+        method: "peek",
+        class_filter: None,
+        runtime: "js_lru_cache_peek",
+        args: &[NA_F64],
+        ret: NR_F64,
+    },
     // ========== commander (CLI parsing) ==========
     // `new Command()` is dispatched separately by `lower_builtin_new` so it
     // produces a real CommanderHandle instead of an empty placeholder. The

crates/perry-ext-lru-cache/Cargo.toml

@@ -17,3 +17,12 @@ lru = "0.18"
 
 [dev-dependencies]
 perry-ffi = { workspace = true, features = ["runtime-link"] }
+# Direct handle for two things the tests need from the runtime:
+# `tests/gc_survival.rs` forces a minor collection
+# (`perry_runtime::gc::gc_collect_minor`) and drives the write-barrier /
+# shadow-frame guard, mirroring perry-ext-events' scanner test; the unit
+# tests use `exception::js_call_catching` to assert on the constructor's
+# npm-matching option errors without the throw exiting the process.
+# `default` + `stdlib` keep this copy feature-identical to the shipped
+# runtime (see perry-ext-events/Cargo.toml for the #6303 rationale).
+perry-runtime = { workspace = true, features = ["default", "stdlib"] }