fix(runtime): four more process-globals that contradict the multi-state contract (#739 item 4)

CHANGELOG.md

@@ -171,6 +171,58 @@ All notable changes to EigenScript are documented here.
 
 ### Fixed
 
+- **Four more process-globals that contradict the multi-state contract (#739
+  item 4).** Each is a resource the runtime scopes per state or per thread
+  everywhere else, held in one process-wide slot:
+  - `s_osr_threshold` (`vm.c`) was a *function static* filled by whichever
+    state crossed a back edge first and then frozen for the life of the
+    process, defeating the per-state JIT tuning `eigenscript.h` explicitly
+    advertises ("so two co-located embedded states can tune independently") —
+    entry and iter thresholds were already per-state; only OSR opted out. It
+    now reads `g_osr_threshold` directly. `eigs_jit_get_osr_threshold()`
+    existed only to fill that cache and is deleted; its comment claimed it was
+    read "once per thread", which was never true — it was once per process.
+  - `g_sandbox_active` / `g_sandbox_bytes_used` / `g_sandbox_byte_max` /
+    `g_sandbox_loop_max` are now per-thread. The save/restore in
+    `builtin_sandbox_run` is correct for one thread's nesting, but the premise
+    it documented — "sandbox_run is synchronous" — holds *per thread* and
+    breaks the moment two states run concurrently, which `ext_http` does per
+    connection: one worker entering a sandbox capped every other worker's
+    loops and charged their allocations against its budget.
+  - `g_stream_file` is now per-thread and closed at `eigs_thread_detach`. One
+    `FILE*` per process meant `stream_open`'s unconditional close tore down
+    another state's in-flight stream, and an unclosed stream lost its buffered
+    tail at exit rather than being flushed.
+  - `g_db_conn` moved onto `EigsState`, closed by a new `ext_db_state_destroy`
+    called from `eigs_state_destroy` — the same shape `ext_http_state_destroy`
+    already had. Previously one worker's `db_connect` replaced the connection
+    another worker was querying through, and nothing ever closed it.
+
+  `g_model` was deliberately left process-wide. Its allocations are reachable
+  from a global at exit, so they are still-reachable rather than leaked; the
+  real defect would be cross-state clobber, and per-state transformer weights
+  would multiply memory by the number of states. That is a design decision
+  about model isolation, not a mechanical scope move.
+
+- **A `task_yield` on one thread silently made every other thread return `null`
+  (#739 item 3).** The cooperative scheduler lives on `EigsThread` — tasks are
+  single-threaded by construction — but the suspend request that drives it was
+  a plain global, set at four sites and polled by every `vm_run` on every
+  thread at `CASE(CALL)`. So a `task_yield` on the main thread drove an
+  unrelated OS worker's next call into `vm_suspend_halt`. Having no scheduler
+  of its own, the victim's `task_save_slice(task_current_running())` was handed
+  NULL and saved nothing, and `vm_run` returned NULL mid-evaluation with its
+  frames deliberately undrained (the suspend path preserves them on purpose) —
+  so a spawned worker silently produced `null` instead of its result. Measured:
+  a worker computing 400000 returned `null` on 3 of 3 runs while main was
+  yielding, and 400000 on 3 of 3 with the identical program minus the yields.
+  The flag is now per-thread. It sits on `EigsThread` beside the `task_sched`
+  it drives rather than inside `TaskScheduler` as first sketched, so the hot
+  `CASE(CALL)` poll stays a single load off the already-hot `eigs_current`
+  with no NULL check for the (overwhelmingly common) no-scheduler thread.
+  `tests/test_tasks.eigs` covers it; the suite had no case where cooperative
+  tasks and OS threads ran at the same time.
+
 - **One `exit of N` permanently disabled `try`/`catch` for the rest of the
   process (#739 item 2).** `exit` is deliberately uncatchable: `builtin_exit`
   sets `g_exit_requested` and `CHECK_ERROR` consults it to refuse routing the

docs/BUILTINS.md

@@ -331,7 +331,7 @@ producing tensors too large to materialise in memory.
 
 | Name | Signature | Description |
 |------|-----------|-------------|
-| `stream_open` | `stream_open of ["path", count]` | Open file, write header for `count` float64 values. 1 on success, 0 on failure |
+| `stream_open` | `stream_open of ["path", count]` | Open file, write header for `count` float64 values. 1 on success, 0 on failure. One stream per **thread**: opening a second closes the first, and an unclosed stream is flushed and closed when the thread ends (#739) |
 | `stream_write` | `stream_write of value` | Append one float64 to the open stream. 1 on success, 0 on failure |
 | `stream_close` | `stream_close of null` | Close the stream. 1 on success, 0 on failure |
 

docs/CONCURRENCY.md

@@ -49,6 +49,23 @@ unsafe pattern — unsynchronized read-modify-write on a shared list from two
 workers — is caught by the ThreadSanitizer gate below
 (`tests/tsan_seeded_race.eigs`).
 
+## Cooperative tasks are per-thread
+
+`task_spawn`/`task_yield` (#408) are a *different* model from `spawn`:
+one OS thread, round-robin, deterministic by construction. The two
+compose, and the scoping is what makes that safe — the scheduler, its
+ready queue, and the suspend request that drives it all live on
+`EigsThread`. A `task_yield` hands control to the next task **on the
+calling thread only**; a `spawn`ed worker with no tasks of its own is
+never suspended by someone else's yield.
+
+That last part was a bug until #739: the suspend request was a plain
+global polled by every `vm_run` at `CASE(CALL)`, so one thread's
+`task_yield` sent an unrelated worker into the suspend path. Having no
+scheduler, the victim saved no slice and its `vm_run` returned `null`
+mid-evaluation — a worker silently produced `null` instead of its
+result. `tests/test_tasks.eigs` now runs both models at once.
+
 ## The multithreaded performance cliff
 
 The first `spawn` in a program permanently flips the runtime into

docs/EMBEDDING.md

@@ -254,6 +254,17 @@ act between evals. While a replay tape is set, nondet builtins return
 the recorded `N` values in order instead of consulting their live
 sources; when the tape runs out they fall back to live.
 
+**Per-state resources are released at `eigs_state_destroy`** (#739). The
+HTTP server's route table and the libpq connection are owned by the
+state that created them and torn down with it
+(`ext_http_state_destroy`, `ext_db_state_destroy`). Sandbox budgets,
+the `stream_open` file handle, the JIT's OSR threshold, and the
+cooperative task scheduler are per-**thread** and released at
+`eigs_thread_detach`. Two co-located states therefore do not share a
+sandbox budget, close each other's stream, or freeze each other's JIT
+tuning — all of which they did while those lived in process globals.
+The trace tape and sink remain per-process by design (below).
+
 **A script's `exit` does not outlive its eval** (#739). `exit of N` is
 deliberately uncatchable — `CHECK_ERROR` refuses to route it to a `try`
 handler — but the request is per-thread and cleared at the top of

docs/SPEC.md

@@ -1255,6 +1255,14 @@ result (or re-raises its uncaught error as the same `{kind, message,
 line}` dict — see [Error handling](#error-handling)). `task_alive of
 id` is 1 until the task finishes.
 
+Tasks are scoped to the OS thread that spawned them: each thread has its
+own scheduler and its own ready queue, so `task_yield` hands control to
+the next task **on this thread** and never disturbs another. Mixing the
+two models is therefore well-defined — a `spawn`ed worker runs its own
+program while the parent interleaves tasks. (Before #739 the suspend
+request was process-wide, so one thread's `task_yield` made every other
+thread's next call return `null` mid-evaluation.)
+
 ```eigenscript
 order is []
 define step(tag) as:

src/builtins.c

@@ -2067,7 +2067,9 @@ Value* builtin_seed_random(Value *arg) {
  *         then count × float64 values (written one at a time via stream_write)
  */
 
-static FILE *g_stream_file = NULL;
+/* g_stream_file is a per-thread bridge macro (eigenscript.h, #739): one FILE*
+ * per PROCESS meant stream_open's unconditional close tore down another
+ * state's in-flight stream. */
 
 #if !EIGENSCRIPT_FREESTANDING
 Value* builtin_stream_open(Value *arg) {

src/eigenscript.h

@@ -588,6 +588,13 @@ struct EigsState {
      * run yet. Carried as an opaque pointer so eigenscript.h stays
      * independent of the ext_http internal layout. */
     struct EigsHttpServer *ext_http_server;
+    /* #739: this state's libpq connection (ext_db.c). Was a process global,
+     * so with a state per HTTP connection one worker's db_connect replaced
+     * the connection another worker was querying through, and nothing ever
+     * closed it — the connection leaked at every state teardown. Opaque
+     * (PGconn*) so this header stays independent of libpq; ext_db_internal.h
+     * defines the accessor. */
+    void                  *ext_db_conn;
 };
 
 struct EigsThread {
@@ -725,6 +732,31 @@ struct EigsThread {
      * first task_spawn, freed at thread detach. Per-OS-thread because tasks
      * are single-threaded by construction. */
     void                *task_sched;
+    /* #739: the suspend request that drives task_sched. Per-OS-thread for the
+     * same reason the scheduler is — it was a plain global, so a task_yield on
+     * one thread drove EVERY other thread's next CALL into vm_suspend_halt:
+     * with no scheduler of its own the victim saved nothing and its vm_run
+     * silently returned NULL mid-evaluation, frames deliberately undrained.
+     * Lives here rather than inside TaskScheduler so the CASE(CALL) poll stays
+     * one load off the already-hot eigs_current, with no NULL check. */
+    int                  task_suspend_request;
+    /* #739: sandbox_run's caps and budget. Per-OS-thread: the save/restore in
+     * builtin_sandbox_run is correct for one thread's nesting, but the
+     * premise it documented — "sandbox_run is synchronous / single-threaded" —
+     * is true per-thread and false the moment two states run concurrently,
+     * which ext_http does per connection. As process globals, one worker
+     * entering a sandbox capped every other worker's loops and charged their
+     * allocations against its budget. */
+    int                  sandbox_loop_max;   /* 0 = default 100M */
+    int                  sandbox_active;
+    size_t               sandbox_bytes_used;
+    size_t               sandbox_byte_max;   /* 0 = no budget */
+    /* #739: stream_open/stream_write/stream_close target. Per-OS-thread: it
+     * was one FILE* per process and stream_open unconditionally closes
+     * whatever is already open, so two states streaming at once closed each
+     * other's file mid-write. Closed at thread detach so an unclosed stream
+     * is not leaked. Opaque (FILE*) to keep stdio out of this header. */
+    void                *stream_file;
     /* Registry list — set by eigs_thread_attach. */
     EigsThread *next;
 };
@@ -808,6 +840,12 @@ extern __thread EigsThread *eigs_current;
 #define g_json_depth          (eigs_current->json_depth)
 #define g_native_call_depth   (eigs_current->native_call_depth)
 #define g_task_sched          (eigs_current->task_sched)
+#define g_task_suspend_request (eigs_current->task_suspend_request)
+#define g_sandbox_loop_max    (eigs_current->sandbox_loop_max)
+#define g_sandbox_active      (eigs_current->sandbox_active)
+#define g_sandbox_bytes_used  (eigs_current->sandbox_bytes_used)
+#define g_sandbox_byte_max    (eigs_current->sandbox_byte_max)
+#define g_stream_file         (*(FILE **)&eigs_current->stream_file)
 #define g_entry_threshold     (eigs_current->state->jit_entry_threshold)
 #define g_iter_threshold      (eigs_current->state->jit_iter_threshold)
 #define g_osr_threshold       (eigs_current->state->jit_osr_threshold)

src/ext_db.c

@@ -7,7 +7,6 @@
 #include "ext_names.h"
 #if EIGENSCRIPT_EXT_DB
 
-PGconn *g_db_conn = NULL;
 
 #define DB_MAX_PARAMS 16
 
@@ -163,6 +162,14 @@ Value* builtin_db_query_json(Value *arg) {
     return result;
 }
 
+/* #739: close this state's connection at teardown. Nothing closed it before,
+ * so every state that ever called db_connect leaked its PGconn. */
+void ext_db_state_destroy(EigsState *st) {
+    if (!st || !st->ext_db_conn) return;
+    PQfinish((PGconn *)st->ext_db_conn);
+    st->ext_db_conn = NULL;
+}
+
 void register_db_builtins(Env *env) {
     /* Expanded from ext_names.h — the shared name list the linter's E003
      * binding base also expands, so registration and name-resolution

src/ext_db_internal.h

@@ -9,8 +9,14 @@
 #include "eigenscript.h"
 #include <libpq-fe.h>
 
-extern PGconn *g_db_conn;
+/* #739: per-STATE connection, reached through the attached thread — the same
+ * shape ext_http's per-state Server uses. Defined here rather than in
+ * eigenscript.h so libpq's types stay out of the core header. */
+#define g_db_conn (*(PGconn **)&eigs_current->state->ext_db_conn)
 
 void register_db_builtins(Env *env);
 
+/* Closes this state's connection; called from eigs_state_destroy. */
+void ext_db_state_destroy(EigsState *st);
+
 #endif

src/jit.c

@@ -2196,12 +2196,6 @@ void jit_state_init_thresholds(EigsState *st) {
     if (st->jit_osr_threshold < 1) st->jit_osr_threshold = 1;
 }
 
-/* Exposed so vm.c can read the OSR threshold once per thread on the
- * first back-edge rather than getenv-checking inside the hot loop. */
-int eigs_jit_get_osr_threshold(void) {
-    return g_osr_threshold;
-}
-
 /* Phase 2b: the body of jit_try_compile_chunk is now this static helper,
  * parameterized on entry_offset and on output pointers for the
  * state/code/advance/stop_op slots so the same emitter can drive both

src/jit.h

@@ -102,7 +102,6 @@ void jit_state_init_thresholds(struct EigsState *st);
 /* OSR threshold accessor for vm.c. vm.c caches the result thread-local
  * on the first JUMP_BACK so the dispatch loop doesn't pay a getenv per
  * iteration. The value is honors-EIGS_JIT_OSR_THRESHOLD env var. */
-int eigs_jit_get_osr_threshold(void);
 
 /* Register a chunk so its exec_count can be dumped at shutdown when
  * EIGS_JIT_HOT=1. Idempotent. Called from vm_run on top-level entry

src/state.c

@@ -12,6 +12,10 @@
  * pthread/socket includes) into core runtime TUs. Defined in ext_http.c. */
 extern void ext_http_state_destroy(EigsState *st);
 #endif
+#if EIGENSCRIPT_EXT_DB
+/* Same reason — declared here rather than including libpq. #739. */
+extern void ext_db_state_destroy(EigsState *st);
+#endif
 
 __thread EigsThread *eigs_current = NULL;
 
@@ -43,6 +47,9 @@ void eigs_state_destroy(EigsState *st) {
 #if EIGENSCRIPT_EXT_HTTP
     /* No-op if the state never registered http builtins. */
     ext_http_state_destroy(st);
+#endif
+#if EIGENSCRIPT_EXT_DB
+    ext_db_state_destroy(st);   /* #739: close this state's libpq connection */
 #endif
     /* Module-cache refs were dropped at gc_collect_at_exit; the array
      * itself may still be allocated (capacity bumped past zero). */
@@ -147,6 +154,13 @@ void eigs_thread_detach(void) {
     vm_thread_destroy(th);
     task_sched_thread_free();   /* #408: release the cooperative task scheduler */
     trace_thread_release();     /* #739: this thread's prev-table (NOT the tape) */
+#if !EIGENSCRIPT_FREESTANDING
+    /* #739: an unclosed stream_open belongs to this thread; close it here so
+     * it is neither leaked nor left for another thread to inherit. Carved out
+     * of the freestanding profile with the stream_* builtins themselves —
+     * fclose is not in the allowlist and there is no stream to close there. */
+    if (th->stream_file) { fclose((FILE *)th->stream_file); th->stream_file = NULL; }
+#endif
 
     /* Phase 8: release freelist + intern memory before the EigsThread
      * struct itself goes. Must run while eigs_current still points at th

src/vm.c

@@ -704,11 +704,10 @@ static Value *make_iter_state(Value *iterable) {
  * g_vm fields without indirection through helper functions. */
 #include <stddef.h>
 
-/* Sandbox loop-iteration cap. 0 = use the default 100,000,000 (normal runs are
- * unaffected); builtin_sandbox_run sets a low value to bound untrusted generated
- * code, then restores it. Read by the LOOP_CAP/STALL checks (interp + JIT
- * helpers). A plain process global: sandbox_run is synchronous. */
-int g_sandbox_loop_max = 0;
+/* Sandbox loop-iteration cap (0 = default 100,000,000) and byte budget live on
+ * EigsThread — see eigenscript.h. #739: they were process globals on the
+ * premise "sandbox_run is synchronous", which holds per-thread and breaks the
+ * moment two states run concurrently (ext_http, one state per connection). */
 
 /* Async abort seam (eigs_set_abort_flag): the embedder registers a flag it
  * may set from an interrupt/signal context; the interpreter polls it on
@@ -752,10 +751,6 @@ volatile int *g_vm_abort_flag = &g_vm_abort_never;
  * inputs are charged), and the text_builder growth path — all bounded by the
  * loop-iteration cap × per-op size, not the byte budget. Extending the budget to
  * them is a one-line sandbox_charge() at each; left out to match #292's scope. */
-int    g_sandbox_active     = 0;
-size_t g_sandbox_bytes_used = 0;
-size_t g_sandbox_byte_max   = 0;   /* 0 = no budget */
-
 int sandbox_charge(size_t bytes) {
     if (!g_sandbox_active || g_sandbox_byte_max == 0) return 1;
     /* Overflow-safe: g_sandbox_bytes_used <= g_sandbox_byte_max is an invariant
@@ -3266,8 +3261,13 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume) {
          * loop ran interpreted for good. Failed offsets keep their slot
          * (state 1) so they cannot retry-storm; when all slots are taken,
          * additional loop headers simply stay interpreted. */
-        static int s_osr_threshold = 0;
-        if (s_osr_threshold == 0) s_osr_threshold = eigs_jit_get_osr_threshold();
+        /* #739: read the per-STATE threshold, don't cache it in a function
+         * static. The static was filled by whichever state crossed a back edge
+         * FIRST and then froze process-wide, defeating the per-state JIT tuning
+         * eigenscript.h advertises ("so two co-located embedded states can tune
+         * independently") — entry and iter thresholds are per-state; only OSR
+         * opted out. g_osr_threshold is a bridge macro, so this is one load off
+         * eigs_current, not the getenv the static was avoiding. */
         {
             int t_off = (int)(ip - chunk->code);
             int osr_hit = -1, osr_free = -1;
@@ -3280,7 +3280,7 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume) {
                 }
             }
             if (osr_hit == -1 && osr_free >= 0 &&
-                chunk->back_edge_count >= (uint32_t)s_osr_threshold) {
+                chunk->back_edge_count >= (uint32_t)g_osr_threshold) {
                 jit_try_compile_chunk_osr(chunk, t_off, osr_free);
                 if (chunk->jit_osr[osr_free].state == 2)
                     osr_hit = osr_free;
@@ -5570,8 +5570,6 @@ static Value *vm_run(EigsChunk *chunk, Env *env) {
  * interleaving is a pure function of program order.
  * ======================================================================== */
 
-int g_task_suspend_request = 0;
-
 #define TASK_READY_MAX HANDLE_TABLE_SIZE
 
 typedef struct {