fix(shard): OOM shield — macOS maxmemory guardrail + eviction bypass closure (RSS/CPU wave 2)

CHANGELOG.md

@@ -29,6 +29,80 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   fresh connection on mid-startup connection resets (CI flake: per-shard
   SO_REUSEPORT listeners accept-then-reset during init).
 
+### Fixed — OOM eviction bypass closure (PR #TBD)
+
+- **Cross-shard SPSC write legs now enforce `--maxmemory`.** `Execute`,
+  `MultiExecute`, `PipelineBatch` and their `*Slotted` variants
+  (`src/shard/spsc_handler.rs`) executed writes against the TARGET shard's
+  `Database` directly, bypassing the connection handlers' eviction gate
+  entirely — a scatter-gather write (e.g. a pipeline of individually-routed
+  `SET`s hashing to a remote shard) could grow that shard's memory past
+  `maxmemory` without limit. A new `spsc_eviction_gate` helper mirrors
+  `run_write_eviction_gate` (`handler_monoio/mod.rs`) exactly — same elastic
+  per-shard budget (GAP-1), same spill-vs-plain branching — threaded through
+  `drain_spsc_shared`/`handle_shard_message_shared` and gated by a
+  once-per-drain-cycle `evict_active` snapshot (perf parity with
+  `batch_eviction_active`: zero extra lock acquires when `maxmemory` is
+  unset).
+- **Lua `redis.call`/`redis.pcall` writes now enforce `--maxmemory`.**
+  EVAL/EVALSHA carry no WRITE command flag, so the dispatch-level OOM check
+  never saw them, and the bridge (`src/scripting/bridge.rs`) never ran the
+  check before executing a write inside a script — a tight `redis.call('SET',
+  ...)` loop could write arbitrarily far past the cap. A new
+  `LuaEvictionCtx`, captured by value into the `redis.call`/`redis.pcall`
+  closures at VM-setup time (`setup_lua_vm`, once per shard — zero new
+  `unsafe`, zero per-call allocation), runs the same gate before a WRITE
+  `redis.call` executes; `redis.call` raises the OOM error as a Lua runtime
+  error (script aborts, matching Redis semantics), `redis.pcall` returns it
+  as an `{err = ...}` table. Read-only scripts are unaffected. FCALL-internal
+  writes (`src/scripting/functions.rs`) keep a documented, pre-existing gap
+  (no shard context threaded through the function-library loader) — tracked
+  as a follow-up, not closed by this fix.
+- **Cross-db `COPY ... DB n` eviction confirmed/hardened.** `COPY` carries
+  the generic `W` write flag, so on the cross-shard dispatch arms
+  `handler_monoio` actually uses for remote writes (`ExecuteSlotted`,
+  `PipelineBatchSlotted`) it already hit the same generic `is_write`
+  eviction gate as any other write command — no COPY-specific bypass exists
+  there. The plain `Execute` arm's `MOVE`/`COPY DB` two-database intercept
+  (special-cased ahead of the generic path because both need two `&mut
+  Database` borrows at once — used by `handler_sharded`'s single-key remote
+  dispatch) *was* missed, since it returns before reaching the generic gate;
+  it now runs `spsc_eviction_gate` against the destination db before
+  `copy_core`. Same-shard `MOVE` is left ungated (net-zero — the key leaves
+  the source db as it lands in the destination, same rationale as `DEL`).
+  This Execute-arm dest-gate is inspection-verified as a verbatim mirror of
+  the tested generic gate; no wire-level case isolates the Execute arm
+  specifically (the arm is only reachable via `handler_sharded`'s
+  single-key remote dispatch, not the pipelined path any test in this suite
+  drives) — see the case E caveat below.
+- **Found, NOT fixed (out of scope): cross-shard remote `COPY ... DB n`
+  silently ignores the DB clause.** The two-database intercept above exists
+  only in the plain `Execute` arm, not `ExecuteSlotted`/`PipelineBatchSlotted`
+  — so when the source key hashes to a different shard than the connection
+  (the common case under `handler_monoio`), `COPY ... DB n` falls through to
+  the generic single-db `key_extra::copy` and silently performs a same-db
+  copy instead of a cross-db one (confirmed via manual probe: 4/20 remote
+  `COPY`s landed in the wrong db). This is a data-correctness bug independent
+  of `--maxmemory` — memory growth from the (wrong) same-db copy is still
+  eviction-gated by the generic path above, so it is not a new OOM bypass,
+  but the DB clause itself does not do what it says. `MOVE` shares the same
+  structural setup (its two-database intercept also lives only in the plain
+  `Execute` arm) and likely has the same DB-target defect on the cross-shard
+  path — not independently verified here. Fixing it requires replicating the
+  two-database intercept across every `ShardMessage` arm (or restructuring it
+  as a shared helper reachable from all of them) — out of scope for this
+  fix, flagged as a follow-up covering both `COPY` and `MOVE`.
+- New wire-level regression suite `tests/oom_bypass_closure.rs` (5 cases,
+  both runtimes): direct-SET control (A), cross-shard pipeline (B), Lua EVAL
+  loop (C), read-only EVAL under pressure not blocked (D), cross-shard COPY
+  under pressure (E). B, C and E RED before this fix (E: 0/300 OOM,
+  GREEN after: 104/300). Case E pipelines `COPY`, which dispatches through
+  the same `ExecuteSlotted`/`PipelineBatchSlotted` generic-gate path as case
+  B — its RED/GREEN swing rides the *generic* gate, not the Execute-arm
+  dest-gate above; it confirms COPY drives that gate to OOM, not that the
+  Execute-arm path is independently exercised. D locks the write-only scope
+  of the Lua gate.
+
 ### Added — int8 symmetric ADC for SQ8 vector search (task #13)
 
 - New per-candidate integer dot-product path for SQ8 asymmetric distance

src/config.rs

@@ -1024,10 +1024,26 @@ fn detect_memory_limit_bytes() -> Option<usize> {
     }
 }
 
-/// Non-Linux: no portable, dependency-free memory-limit probe. The guardrail
-/// is skipped (operator sets `--maxmemory` explicitly on dev hosts). Production
-/// targets Linux per the platform policy.
-#[cfg(not(target_os = "linux"))]
+/// macOS (first-class target): probe physical RAM via `sysctl -n hw.memsize`.
+/// A spawned command instead of `sysctlbyname` FFI keeps this free of new
+/// unsafe blocks; it runs once at startup so the fork cost is irrelevant.
+/// No container limit concept applies on macOS, so host RAM is the limit.
+#[cfg(target_os = "macos")]
+fn detect_memory_limit_bytes() -> Option<usize> {
+    let out = std::process::Command::new("sysctl")
+        .args(["-n", "hw.memsize"])
+        .output()
+        .ok()?;
+    if !out.status.success() {
+        return None;
+    }
+    std::str::from_utf8(&out.stdout).ok()?.trim().parse().ok()
+}
+
+/// Other platforms: no portable, dependency-free memory-limit probe. The
+/// guardrail is skipped (operator sets `--maxmemory` explicitly). Production
+/// targets Linux/macOS per the platform policy.
+#[cfg(not(any(target_os = "linux", target_os = "macos")))]
 fn detect_memory_limit_bytes() -> Option<usize> {
     None
 }
@@ -1539,6 +1555,19 @@ mod tests {
         assert_eq!(config.to_runtime_config().maxmemory, 0);
     }
 
+    #[test]
+    #[cfg(target_os = "macos")]
+    fn macos_detects_memory_limit() {
+        // macOS is a first-class target; booting UNLIMITED + noeviction by
+        // default (guardrail Skipped) was RSS/CPU/OOM review item 4 — the
+        // hw.memsize probe must feed the same 80% guardrail as Linux.
+        let detected = detect_memory_limit_bytes();
+        assert!(
+            detected.is_some_and(|b| b > 1 << 30),
+            "hw.memsize probe failed: {detected:?}"
+        );
+    }
+
     #[test]
     fn parse_meminfo_extracts_memtotal_bytes() {
         let sample = "MemTotal:       16384256 kB\nMemFree:    1000 kB\n";

src/scripting/bridge.rs

@@ -7,11 +7,133 @@
 //! 3. The pointer is cleared immediately after script execution
 
 use std::cell::Cell;
+use std::path::PathBuf;
+use std::rc::Rc;
+use std::sync::Arc;
 
 use mlua::prelude::*;
 
+use crate::config::RuntimeConfig;
 use crate::protocol::Frame;
+use crate::shard::shared_databases::ShardDatabases;
 use crate::storage::engine::StorageEngine;
+use crate::storage::eviction::{
+    try_evict_if_needed_async_spill_budget, try_evict_if_needed_budget,
+};
+use crate::storage::tiered::spill_thread::SpillRequest;
+
+/// Shard context needed to enforce `--maxmemory` eviction before a Lua
+/// `redis.call`/`redis.pcall` WRITE actually mutates the database.
+///
+/// # Why closure-capture instead of a thread-local
+///
+/// The bridge already uses a thread-local raw pointer (`CURRENT_DB`) for the
+/// `Database` itself, because that pointer's target changes on every script
+/// invocation. This context is different: it is the same for every script run
+/// on a given shard for the shard's entire lifetime (the shard's
+/// `ShardDatabases`/`RuntimeConfig`/spill handles never change identity).
+/// `redis.call`/`redis.pcall` are Lua closures created exactly once per shard
+/// by [`crate::scripting::setup_lua_vm`], at a point where the caller already
+/// owns cloneable handles to all of this — so it is captured directly into
+/// the `move` closure. This adds zero new `unsafe` code (the existing
+/// `CURRENT_DB` unsafe deref is untouched) and zero per-call allocation. The
+/// common case (`maxmemory` unset, no spill) is decided by a per-script
+/// snapshot (`evict_active` below), so a tight `redis.call('SET', ...)` loop
+/// pays one `Cell` read per write call — the `RuntimeConfig` lock is read at
+/// most once per script execution, mirroring the per-batch
+/// `batch_eviction_active` snapshot the connection handlers use.
+#[derive(Clone)]
+pub struct LuaEvictionCtx(Option<LuaEvictionInner>);
+
+#[derive(Clone)]
+struct LuaEvictionInner {
+    shard_databases: Arc<ShardDatabases>,
+    runtime_config: Arc<parking_lot::RwLock<RuntimeConfig>>,
+    shard_id: usize,
+    spill_sender: Option<flume::Sender<SpillRequest>>,
+    spill_file_id: Rc<Cell<u64>>,
+    disk_offload_dir: Option<PathBuf>,
+    /// `(script generation, evict-active)` snapshot, refreshed at most once
+    /// per script execution (generation bumped by `set_script_db`). Sentinel
+    /// generation `u64::MAX` forces a refresh on first use. Staleness is
+    /// bounded to one script run — same bound as the handlers' per-batch
+    /// snapshot.
+    evict_active: Cell<(u64, bool)>,
+}
+
+impl LuaEvictionCtx {
+    /// No-op gate. Used by unit tests (no real shard context available) and
+    /// by the FCALL path (`src/scripting/functions.rs`), which has a
+    /// pre-existing, documented gap for in-function write eviction — closing
+    /// it is out of scope for this fix (see `tmp/OOM-SHIELD-CONTEXT.md`).
+    pub fn disabled() -> Self {
+        LuaEvictionCtx(None)
+    }
+
+    /// Real gate, built from the shard's own handles at VM-setup time.
+    #[allow(clippy::too_many_arguments)]
+    pub fn new(
+        shard_databases: Arc<ShardDatabases>,
+        runtime_config: Arc<parking_lot::RwLock<RuntimeConfig>>,
+        shard_id: usize,
+        spill_sender: Option<flume::Sender<SpillRequest>>,
+        spill_file_id: Rc<Cell<u64>>,
+        disk_offload_dir: Option<PathBuf>,
+    ) -> Self {
+        LuaEvictionCtx(Some(LuaEvictionInner {
+            shard_databases,
+            runtime_config,
+            shard_id,
+            spill_sender,
+            spill_file_id,
+            disk_offload_dir,
+            evict_active: Cell::new((u64::MAX, false)),
+        }))
+    }
+
+    /// Run the same eviction/OOM gate the connection handlers use
+    /// (`run_write_eviction_gate` in `handler_monoio/mod.rs`), against `db`
+    /// (the shard's `Database`, already borrowed by the caller via the
+    /// `CURRENT_DB` thread-local). Returns the standard OOM `Frame::Error`
+    /// on failure; `Ok(())` if within budget, eviction succeeded, or
+    /// `maxmemory` is unset.
+    fn gate(&self, db: &mut crate::storage::Database, db_index: usize) -> Result<(), Frame> {
+        let Some(inner) = self.0.as_ref() else {
+            return Ok(());
+        };
+        // Lock-free fast path: decide "is eviction active at all?" from a
+        // per-script snapshot so a script issuing thousands of writes takes
+        // the RuntimeConfig read lock once, not per redis.call.
+        let generation = SCRIPT_GENERATION.with(|c| c.get());
+        let (cached_generation, cached_active) = inner.evict_active.get();
+        let active = if cached_generation == generation {
+            cached_active
+        } else {
+            let active = inner.spill_sender.is_some() || inner.runtime_config.read().maxmemory != 0;
+            inner.evict_active.set((generation, active));
+            active
+        };
+        if !active {
+            return Ok(());
+        }
+        let rt = inner.runtime_config.read();
+        let budget = inner.shard_databases.elastic_budget(inner.shard_id);
+        if let Some(sender) = &inner.spill_sender {
+            let mut fid = inner.spill_file_id.get();
+            let dir = inner
+                .disk_offload_dir
+                .as_deref()
+                .unwrap_or(std::path::Path::new("."));
+            let res = try_evict_if_needed_async_spill_budget(
+                db, &rt, sender, dir, &mut fid, db_index, budget,
+            );
+            inner.spill_file_id.set(fid);
+            res
+        } else {
+            try_evict_if_needed_budget(db, &rt, budget)
+        }
+    }
+}
 
 thread_local! {
     /// Raw pointer to the current shard's Database during script execution.
@@ -24,6 +146,10 @@ thread_local! {
     static SCRIPT_HAD_WRITE: Cell<bool> = const { Cell::new(false) };
     /// Whether this script is running in read-only mode (FCALL_RO).
     static SCRIPT_READ_ONLY: Cell<bool> = const { Cell::new(false) };
+    /// Monotonic script-execution counter, bumped by `set_script_db`, so
+    /// per-VM caches (`LuaEvictionCtx::evict_active`) can detect a new
+    /// script run without taking any lock.
+    static SCRIPT_GENERATION: Cell<u64> = const { Cell::new(0) };
 }
 
 /// Set the thread-local database pointer before script execution.
@@ -32,6 +158,7 @@ pub fn set_script_db(db: &mut crate::storage::Database, db_idx: usize, db_count:
     CURRENT_DB_IDX.with(|c| c.set(db_idx));
     CURRENT_DB_COUNT.with(|c| c.set(db_count));
     SCRIPT_HAD_WRITE.with(|c| c.set(false));
+    SCRIPT_GENERATION.with(|c| c.set(c.get().wrapping_add(1)));
 }
 
 /// Clear the thread-local database pointer after script execution.
@@ -59,7 +186,11 @@ pub fn script_had_write() -> bool {
 ///
 /// If `propagate_errors` is true (redis.call), Frame::Error results are raised as Lua errors.
 /// If false (redis.pcall), errors are returned as {err = "..."} tables.
-pub fn make_redis_call_fn(lua: &Lua, propagate_errors: bool) -> mlua::Result<LuaFunction> {
+pub fn make_redis_call_fn(
+    lua: &Lua,
+    propagate_errors: bool,
+    eviction_ctx: LuaEvictionCtx,
+) -> mlua::Result<LuaFunction> {
     lua.create_function(move |lua, args: LuaMultiValue| {
         // Convert all Lua arguments to Frames
         let frames: Vec<Frame> = args
@@ -110,6 +241,14 @@ pub fn make_redis_call_fn(lua: &Lua, propagate_errors: bool) -> mlua::Result<Lua
             if cmd_is_write {
                 // Track writes for SCRIPT KILL safety check
                 SCRIPT_HAD_WRITE.with(|c| c.set(true));
+                // OOM eviction gate (M3): mirrors the connection handlers'
+                // `run_write_eviction_gate` — without this, a write inside a
+                // script could grow memory past `maxmemory` without limit
+                // (EVAL/EVALSHA carry no WRITE command flag, so the
+                // dispatch-level OOM check never sees them at all).
+                if let Err(oom) = eviction_ctx.gate(db, db_idx) {
+                    return Ok(oom);
+                }
             }
 
             let frame = db.execute_command(&cmd_bytes, &frames[1..], &mut db_idx, db_count);

src/scripting/functions.rs

@@ -283,8 +283,14 @@ impl FunctionRegistry {
         let lua = Rc::new(mlua::Lua::new());
         crate::scripting::sandbox::setup_sandbox(&lua)
             .map_err(|e| LoadError::LuaError(e.to_string()))?;
-        crate::scripting::sandbox::register_redis_api(&lua)
-            .map_err(|e| LoadError::LuaError(e.to_string()))?;
+        // FCALL-internal writes: pre-existing, documented gap (no shard
+        // context is threaded through the function-library loader). Closing
+        // it is out of scope here — see tmp/OOM-SHIELD-CONTEXT.md.
+        crate::scripting::sandbox::register_redis_api(
+            &lua,
+            crate::scripting::bridge::LuaEvictionCtx::disabled(),
+        )
+        .map_err(|e| LoadError::LuaError(e.to_string()))?;
 
         // Create a table to store registered functions
         let func_table = lua