ci: detect memory misuse before merge, and fix the five lifetime bugs it surfaces

.github/workflows/ci.yml

@@ -11,6 +11,23 @@ env:
 jobs:
   test:
     runs-on: ubuntu-latest
+    strategy:
+      # Both legs must report: a debug-flavour failure is a real memory bug and
+      # cancelling the release leg over it (or vice versa) hides half the story.
+      fail-fast: false
+      matrix:
+        # Which flavour of librayforce.a to link. `debug` compiles the core with
+        # -DDEBUG, arming the engine's stale retain/release detector (DFD) — see
+        # rayforce-sys/build.rs. ASan cannot see into the engine's mmap-backed
+        # pool allocator, so DFD is the only tool for this bug class.
+        core: [release, debug]
+    env:
+      # Job-scoped, not step-scoped: the core binary is built in its own step and
+      # must use the same flavour, or build.rs would see a flavour flip at
+      # `cargo test` time and `make clean` the binary away. Empty on the release
+      # leg — build.rs and the core's dfd_enabled() both read empty as off.
+      RAYFORCE_CORE_DEBUG: ${{ matrix.core == 'debug' && '1' || '' }}
+      RAY_DFD: ${{ matrix.core == 'debug' && '1' || '' }}
     steps:
       # The C core and the rayforce-q client are submodules under
       # rayforce-sys/vendor/, so this one checkout brings the whole build.
@@ -45,14 +62,88 @@ jobs:
             ~/.cargo/registry
             ~/.cargo/git
             target
-          key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.toml') }}
+          key: ${{ runner.os }}-cargo-${{ matrix.core }}-${{ hashFiles('**/Cargo.toml') }}
 
+      # Lints are flavour-independent; running them twice only doubles the wait.
       - name: Format check
+        if: matrix.core == 'release'
         run: cargo fmt --all -- --check
 
       - name: Clippy
+        if: matrix.core == 'release'
         run: cargo clippy --workspace --all-targets -- -D warnings
 
+      # tests/ipc.rs spawns a real server to exercise TcpClient — the path
+      # Runtime's poll teardown lives on. Without a binary it returns early and
+      # reports as a pass, so that coverage was silently absent from CI.
+      #
+      # Ordering matters: `cargo build` runs build.rs, which stages the vendored
+      # core into OUT_DIR and runs `make lib` there at this leg's flavour. That
+      # staged tree is what `make release` is pointed at, so it reuses those
+      # objects untouched and only adds main.c plus the link — a quarter second,
+      # not a second engine build. On the debug leg the engine objects carry DFD,
+      # so the server checks its own side of every exchange. (main.c itself is
+      # compiled at the Makefile's default RELEASE_CFLAGS either way. `-DDEBUG`
+      # reaches no struct in the public header — only the RAY_ASSERT_VALUE macro
+      # at rayforce.h:216 — so the two agree on layout.)
+      #
+      # The three make variables are read back out of build.rs rather than
+      # restated, the way scripts/check-vendored-pin.sh already reads them: the
+      # vendored build drops -Werror on purpose, and `git` resolving RAY_VERSION
+      # from inside OUT_DIR would stamp this repository's HEAD into the server
+      # binary instead of the core's.
+      - name: Build the core server binary
+        run: |
+          cargo build --workspace --tests
+          # Absolute: cargo runs each test binary with the cwd set to its own
+          # crate root, so a workspace-relative RAYFORCE_BINARY would not resolve.
+          CORE=$(ls -dt "$PWD"/target/debug/build/rayforce-sys-*/out/core 2>/dev/null | head -1)
+          if [ ! -f "$CORE/librayforce.a" ]; then
+            echo "::error::no staged core under target/debug/build — did build.rs run?"; exit 1
+          fi
+          field() { sed -n "s/^const $1: &str = \"\(.*\)\";\$/\1/p" rayforce-sys/build.rs; }
+          make -C "$CORE" release \
+            WARNS="$(field CORE_WARNS)" \
+            RAY_VERSION="$(field CORE_VERSION)" \
+            GIT_HASH="$(field CORE_COMMIT)"
+          echo "RAYFORCE_BINARY=$CORE/rayforce" >> "$GITHUB_ENV"
+          echo "RAYFORCE_REQUIRE_SERVER=1" >> "$GITHUB_ENV"
+          echo "RAYFORCE_CORE_DIR=$CORE" >> "$GITHUB_ENV"
+
+      # The debug leg's whole purpose is that -DDEBUG reached the archive, and
+      # nothing checked that it did. A break in the RAYFORCE_CORE_DEBUG plumbing
+      # — an env rename, a build.rs refactor, a change in how Actions evaluates
+      # `&& '1' || ''` — would turn this leg into a second release run that stays
+      # green forever: the same shape as the tests/ipc.rs early return the step
+      # above closes.
+      #
+      # Both legs assert, in opposite directions. Without the release-leg
+      # control, a grep that matched nothing anywhere would pass too, which is
+      # precisely the failure being guarded against.
+      #
+      # This proves the detector is compiled in, not that it is armed. Arming is
+      # dfd_enabled() reading RAY_DFD (the core's src/mem/heap.c); the only proof
+      # of that is a deliberate double release, which aborts the process and so
+      # cannot live in the suite.
+      - name: Check the DFD detector matches the core flavour
+        run: |
+          # `grep -c`, not `grep -q`: `-q` exits on the first match, `nm` then
+          # takes SIGPIPE, and under `pipefail` the pipeline reports failure —
+          # so a present symbol reads as absent. That inverts this check into
+          # one that passes for the wrong reason on the release leg, which is
+          # the bug class the whole step exists to catch. Observed locally.
+          n=$(nm "$RAYFORCE_CORE_DIR/librayforce.a" | grep -c ray_dfd_check_live || true)
+          if [ "${{ matrix.core }}" = debug ]; then
+            if [ "$n" -eq 0 ]; then
+              echo "::error::debug archive carries no ray_dfd_check_live — DFD did not compile in, so this leg detects nothing"
+              exit 1
+            fi
+          elif [ "$n" -ne 0 ]; then
+            echo "::error::release archive carries ray_dfd_check_live ($n) — this leg built the debug flavour"
+            exit 1
+          fi
+          echo "${{ matrix.core }} archive: $n ray_dfd_check_live symbols, as expected"
+
       - name: Test
         run: cargo test --workspace
 

README.md

@@ -51,28 +51,30 @@ copied element by element.
 ```rust
 use rayforce::{col, Runtime, Table, Value};
 
-let _rt = Runtime::new()?;                       // one live runtime per process
-
-let quotes = Table::new(
-    &["symbol", "bid", "ask"],
-    &[
-        Value::sym_vec(&["AAPL", "AAPL", "AAPL", "GOOG", "GOOG", "GOOG"]),
-        Value::vec(&[100.0f64, 101.0, 102.0, 200.0, 201.0, 202.0]),
-        Value::vec(&[110.0f64, 111.0, 112.0, 210.0, 211.0, 212.0]),
-    ],
-)?;
-
-let result = quotes
-    .select()
-    .agg("max_bid", col("bid").max())
-    .agg("min_bid", col("bid").min())
-    .agg("avg_ask", col("ask").avg())
-    .agg("count",   col("bid").count())
-    .filter(col("bid").ge(110.0).and(col("ask").gt(100.0)))
-    .by("symbol")
-    .execute()?;
-
-println!("{result}");
+// One live runtime per process; the scope brackets its whole life.
+Runtime::scope(|_rt| {
+    let quotes = Table::new(
+        &["symbol", "bid", "ask"],
+        &[
+            Value::sym_vec(&["AAPL", "AAPL", "AAPL", "GOOG", "GOOG", "GOOG"]),
+            Value::vec(&[100.0f64, 101.0, 102.0, 200.0, 201.0, 202.0]),
+            Value::vec(&[110.0f64, 111.0, 112.0, 210.0, 211.0, 212.0]),
+        ],
+    )?;
+
+    let result = quotes
+        .select()
+        .agg("max_bid", col("bid").max())
+        .agg("min_bid", col("bid").min())
+        .agg("avg_ask", col("ask").avg())
+        .agg("count",   col("bid").count())
+        .filter(col("bid").ge(110.0).and(col("ask").gt(100.0)))
+        .by("symbol")
+        .execute()?;
+
+    println!("{result}");
+    Ok(())
+})?;
 ```
 
 ```text
@@ -146,6 +148,12 @@ export RAYFORCE_SRC=/path/to/rayforce
 export RAYFORCE_Q_SRC=/path/to/rayforce-q
 ```
 
+Such a checkout is built in place, so incremental state is preserved — except across a
+core-flavour switch. Release and debug objects share every filename, so the first build
+after `RAYFORCE_CORE_DEBUG` changes drops every object under `src/` and the
+`librayforce.a` beside them, and records the flags in an untracked `.stamp`. Nothing
+tracked by git is touched.
+
 `bindgen` locates `libclang` via `LIBCLANG_PATH`. This is deliberately **not** set in the
 repo's `.cargo/config.toml`. If bindgen can't auto-detect libclang, set it yourself:
 

docs/docs/content/CHANGELOG.md

@@ -3,6 +3,116 @@
 All notable changes to `rayforce` are documented here. This project adheres to
 [Semantic Versioning](https://semver.org).
 
+## Unreleased
+
+### Added
+
+- **CI runs the suite against a debug-flavour engine.** Set
+  `RAYFORCE_CORE_DEBUG=1` and `rayforce-sys` builds `librayforce.a` with
+  `-DDEBUG`, which compiles in the core's invariant checks and its stale
+  retain/release detector; arm it at runtime with `RAY_DFD=1`. This is the only
+  tool that sees a use-after-free inside the engine's `mmap`-backed pool
+  allocator — AddressSanitizer and Valgrind track `malloc`, which the engine
+  never calls, and Miri cannot execute the C library at all. The `test` job now
+  runs both flavours; the debug leg reproduces the `Value`-outliving-`Runtime`
+  crash below on the commit before its fix. Both legs then assert the archive
+  they built: `ray_dfd_check_live` must be present on the debug leg and absent on
+  the release one. Without that pair, a break in the `RAYFORCE_CORE_DEBUG`
+  plumbing would turn the debug leg into a second release run that stays green.
+
+- **The IPC tests run in CI.** `tests/ipc.rs` drives `TcpClient` against a
+  spawned server and was returning early for want of one — which reports as a
+  pass, so the gap was invisible. CI now builds the server binary, and
+  `RAYFORCE_REQUIRE_SERVER=1` turns a missing one into a failure rather than a
+  skip. `tests/q_real.rs` still opts out via `RAYFORCE_Q_ADDR`: it needs a real
+  `q` server, which cannot be provisioned on a runner.
+
+### Changed
+
+- **A core-flavour switch rebuilds a `RAYFORCE_SRC` checkout from scratch.**
+  Release and debug objects share every filename and `make` tracks headers but
+  not flags, so a flavour flip would otherwise archive a mixed library. The
+  build script now drops every object under the core's `src/` and the
+  `librayforce.a` beside them on the first build after the flags change, and
+  records them in an untracked `.stamp` file — in your own checkout as well as
+  under `OUT_DIR`, which previously had the only such check. Nothing tracked by
+  git is touched.
+
+- **Breaking: `Runtime::scope` replaces `Runtime::new`.** `Runtime::new` is
+  private; the only way to a runtime is
+  `Runtime::scope(|rt| { … })`, which creates it, hands the closure a
+  `&Runtime` you cannot drop or move out of, and tears it down when the closure
+  returns — on the error path and on unwind alike. A nested scope errors rather
+  than starting a second runtime. Migration is mechanical: delete
+  `let _rt = Runtime::new()?;`, wrap the body, end it with `Ok(())`.
+
+- **Nothing engine-backed leaves a scope.** `Runtime::scope` requires `Send` of
+  its return type and of the closure, and `Value`, `Table`, `Fn`, `TcpClient`
+  and `QConnection` are all `!Send` — so returning one, or assigning one into a
+  variable declared outside, is a compile error reading `required by a bound in
+  Runtime::scope`. The cost is that an unrelated `!Send` capture (an `Rc`, a
+  `RefCell` borrow) is refused too, with a diagnostic about threads when no
+  thread is involved; construct such values inside the closure, or move them in.
+
+- **Breaking: `is_live()` is now `on_runtime_thread()`**, and answers a
+  per-thread question rather than a per-process one. A live runtime is required
+  for everything except reading and dropping handles you already hold: `eval`,
+  `set_global`, `get_global`, the value constructors and the connection
+  constructors all answer to this one predicate, which is true only inside a
+  scope *and* only on the thread that entered it. A `false` result does not mean
+  a runtime can be created — one may be live on another thread, and
+  `Runtime::scope` says so.
+
+### Fixed
+
+- **Engine calls from another thread are refused instead of segfaulting.** The
+  liveness flag was a process-wide `AtomicBool`, but everything it guards is
+  thread-local: the core's VM (`__VM`) and heap (`ray_tl_heap`) both are. So
+  inside a scope, any other thread saw a live runtime and every guard passed —
+  `std::thread::spawn(|| rayforce::eval("(+ 1 1)"))` crashed in `ray_eval_str`,
+  which dereferences `__VM` with no null check, from safe code with no `unsafe`
+  anywhere. Constructors were quieter but not better: off-thread
+  `Value::sym("hello")` succeeded, allocating into a per-thread heap that no
+  `ray_runtime_destroy` would ever unmap. The guard is now a thread-local, so
+  those calls panic naming the thread; creating a runtime stays process-wide,
+  because the core's `__RUNTIME` is an unguarded global that a second
+  `ray_runtime_create` would overwrite in silence.
+
+- **A `Value` can no longer outlive its `Runtime`.** Dropping the runtime
+  unmaps the engine heap, so a handle still alive afterwards released into
+  memory that is no longer mapped. No check at the point of use could have
+  helped: `ray_t.rc` counts references to an *object*, while
+  `ray_runtime_destroy` munmaps every pool without consulting it, and by the
+  time a stale handle is used the thing to check is the pointer — which is what
+  became invalid. `Runtime::scope` removes the shape instead: the closure's
+  locals are dropped before the runtime is, and its `Send` bounds stop a value
+  leaving. `Value` stays one pointer wide, with no bookkeeping on clone or drop.
+
+- **The connection types are confined to their scope too.** `TcpClient` and
+  `QConnection` had no liveness tracking of any kind, so a client outliving its
+  `Runtime` called `ray_ipc_close` / `q_close` against an unmapped heap. Both
+  are now `!Send`/`!Sync` with `compile_fail` markers pinning it, which is what
+  the scope's bounds read, and both `Drop`s run before the runtime's.
+
+- **Building a value requires a live `Runtime`.** `Value::i64(1)` with no runtime
+  was safe Rust calling straight into the engine with no check at all. It did not
+  crash, which is why it went unnoticed: `ray_alloc` lazily maps a heap when none
+  exists, so the value landed in an orphan one. The sharp case was symbols, which
+  are runtime-scoped — `Value::sym("hello")` returned an *empty* symbol, dropping
+  the string with no error anywhere.
+
+- **The runtime tears down its event loop.** `TcpClient::connect` installs a
+  poll on first use and `ray_runtime_destroy` does not touch it, so it leaked.
+  `Runtime`'s `Drop` now takes it down first, while the heap it releases
+  selector state into is still there.
+
+- **`QConnection` is `!Send`/`!Sync`**, like every other handle in the crate.
+  It was a bare file descriptor, so it inferred both, while `execute` interns
+  symbols and builds engine objects that belong to the runtime's thread.
+
+- Building with `--no-default-features` (no `chrono`) is now warning-free.
+
+
 ## 1.0.1
 
 ### Added

docs/docs/content/documentation/data-types/boolean.md

@@ -6,7 +6,8 @@ with `Value::bool_vec`.
 !!! note "Assume a live runtime"
     ```rust
     use rayforce::{Runtime, Value};
-    let _rt = Runtime::new()?;
+    // every snippet below runs inside:
+    Runtime::scope(|rt| { /* … */ })?;
     ```
 
 ## Atoms

docs/docs/content/documentation/data-types/dict.md

@@ -8,7 +8,8 @@ block of a [table](../table/overview.md).
 !!! note "Assume a live runtime"
     ```rust
     use rayforce::{Runtime, Value};
-    let _rt = Runtime::new()?;
+    // every snippet below runs inside:
+    Runtime::scope(|rt| { /* … */ })?;
     ```
 
 ## Construction

docs/docs/content/documentation/data-types/float.md

@@ -6,7 +6,8 @@ double-precision `F64`.
 !!! note "Assume a live runtime"
     ```rust
     use rayforce::{Runtime, Value};
-    let _rt = Runtime::new()?;
+    // every snippet below runs inside:
+    Runtime::scope(|rt| { /* … */ })?;
     ```
 
 ## Atoms

docs/docs/content/documentation/data-types/functions.md

@@ -8,7 +8,8 @@ wraps a compiled function object built from Rayfall source — for example
 !!! note "Assume a live runtime"
     ```rust
     use rayforce::{Fn, Runtime, Value};
-    let _rt = Runtime::new()?;
+    // every snippet below runs inside:
+    Runtime::scope(|rt| { /* … */ })?;
     ```
 
 ## Construction

docs/docs/content/documentation/data-types/guid.md

@@ -6,7 +6,8 @@ for keys that must be unique across machines without coordination.
 !!! note "Assume a live runtime"
     ```rust
     use rayforce::{Runtime, Value, Guid};
-    let _rt = Runtime::new()?;
+    // every snippet below runs inside:
+    Runtime::scope(|rt| { /* … */ })?;
     ```
 
 ## Constructor and reader

docs/docs/content/documentation/data-types/integers.md

@@ -7,7 +7,8 @@ contiguous vector form.
 !!! note "Assume a live runtime"
     ```rust
     use rayforce::{Runtime, Value};
-    let _rt = Runtime::new()?;
+    // every snippet below runs inside:
+    Runtime::scope(|rt| { /* … */ })?;
     ```
 
 ## Atoms

docs/docs/content/documentation/data-types/list.md

@@ -7,7 +7,8 @@ mix atoms, vectors, dicts, or even nested lists.
 !!! note "Assume a live runtime"
     ```rust
     use rayforce::{Runtime, Value};
-    let _rt = Runtime::new()?;
+    // every snippet below runs inside:
+    Runtime::scope(|rt| { /* … */ })?;
     ```
 
 ## Construction

docs/docs/content/documentation/data-types/overview.md

@@ -12,7 +12,8 @@ the payload.
 
     ```rust
     use rayforce::{Runtime, Value};
-    let _rt = Runtime::new()?;
+    // every snippet below runs inside:
+    Runtime::scope(|rt| { /* … */ })?;
     ```
 
 ## The value model

docs/docs/content/documentation/data-types/string.md

@@ -7,7 +7,8 @@ or high-cardinality text.
 !!! note "Assume a live runtime"
     ```rust
     use rayforce::{Runtime, Value, Str};
-    let _rt = Runtime::new()?;
+    // every snippet below runs inside:
+    Runtime::scope(|rt| { /* … */ })?;
     ```
 
 ## Atoms

docs/docs/content/documentation/data-types/symbol.md

@@ -8,7 +8,8 @@ comparison and storage operate on the id, not the characters.
 !!! note "Assume a live runtime"
     ```rust
     use rayforce::{Runtime, Value};
-    let _rt = Runtime::new()?;
+    // every snippet below runs inside:
+    Runtime::scope(|rt| { /* … */ })?;
     ```
 
 ## Atoms