From 5549616628f3b8e8b74da56eb15ffbbb00943395 Mon Sep 17 00:00:00 2001 From: InauguralPhysicist Date: Wed, 29 Jul 2026 01:24:15 -0500 Subject: [PATCH 1/2] fix(tasks): a task_yield on one thread no longer suspends every other (#739 item 3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). A spawned worker therefore produced `null` instead of its result, silently. Reproduced with a control — two workers each computing 400000 while main drives a task_yield loop: workers + main yielding w1=null w2=null (3 of 3 runs) identical program, no yields w1=400000 w2=400000 (3 of 3 runs) The flag is now per-thread. It sits on EigsThread beside the task_sched it drives rather than inside TaskScheduler as the issue sketched: the poll is on the hot CASE(CALL) path, and inside the scheduler it would need `g_task_sched && ((TaskScheduler*)g_task_sched)->suspend_request` — two dependent loads plus a branch, paid by every thread that has no scheduler at all, which is nearly all of them. On EigsThread it stays a single load off the already-hot eigs_current, no NULL check. tests/test_tasks.eigs gains the case; the suite had none where cooperative tasks and OS threads ran at the same time. Validated red: "expected 400000, got null" on both workers without the fix. Gates: release 3274/3274; asan+ubsan 3278/3278, leak tally 0; TSan 11/11 including its seeded-race self-validation (the gate is live); tasks 79/79; freestanding stage 1+2 OK; jit-smoke; embed stack soak OK. Refs #739 Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 19 +++++++++++++++++++ docs/CONCURRENCY.md | 17 +++++++++++++++++ docs/SPEC.md | 8 ++++++++ src/eigenscript.h | 9 +++++++++ src/vm.c | 2 -- src/vm.h | 4 +++- tests/test_tasks.eigs | 27 +++++++++++++++++++++++++++ 7 files changed, 83 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 92c147cb..6c43f0de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -171,6 +171,25 @@ All notable changes to EigenScript are documented here. ### Fixed +- **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 diff --git a/docs/CONCURRENCY.md b/docs/CONCURRENCY.md index 4f8f23b5..3c55941c 100644 --- a/docs/CONCURRENCY.md +++ b/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 diff --git a/docs/SPEC.md b/docs/SPEC.md index 960f5283..eddddc40 100644 --- a/docs/SPEC.md +++ b/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: diff --git a/src/eigenscript.h b/src/eigenscript.h index 4dfe0f89..fce38d0e 100644 --- a/src/eigenscript.h +++ b/src/eigenscript.h @@ -725,6 +725,14 @@ 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; /* Registry list — set by eigs_thread_attach. */ EigsThread *next; }; @@ -808,6 +816,7 @@ 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_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) diff --git a/src/vm.c b/src/vm.c index 456c6777..b20c42d2 100644 --- a/src/vm.c +++ b/src/vm.c @@ -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 { diff --git a/src/vm.h b/src/vm.h index 72bede69..214169d5 100644 --- a/src/vm.h +++ b/src/vm.h @@ -557,7 +557,9 @@ void task_free(Task *t); * above the outermost vm_execute, so C-stack depth stays flat across any * number of task switches. Suspension is deterministic-by-construction — no * tape records. */ -extern int g_task_suspend_request; /* set by a suspending builtin; actioned at CASE(CALL) */ +/* g_task_suspend_request (set by a suspending builtin, actioned at CASE(CALL)) + * is a per-thread bridge macro in eigenscript.h — #739: as a global it drove + * other threads' CALL sites into a suspend they never asked for. */ void task_sched_on_spawn(int id); /* enqueue a freshly spawned task, arm the scheduler */ void task_request_yield(void); /* current task → tail of the ready queue */ int task_request_join(int target); /* current task blocks on `target`; 0 = bad target */ diff --git a/tests/test_tasks.eigs b/tests/test_tasks.eigs index 7b347806..1c2d50f5 100644 --- a/tests/test_tasks.eigs +++ b/tests/test_tasks.eigs @@ -499,4 +499,31 @@ assert_eq of [task_join of _det_t3, null, "self-detached task is reaped at finis assert_eq of [task_detach of 0, 0, "task_detach of main is refused"] assert_eq of [task_detach of 99999, 0, "task_detach of an unknown id is refused"] +# #739 item 3: a task_yield on THIS thread must not suspend another OS thread. +# The suspend request used to be a plain global while the scheduler it drives +# is per-thread, so every other thread's next CALL saw the flag and jumped to +# vm_suspend_halt. Having no scheduler of its own, the victim saved no slice +# and its vm_run returned NULL mid-evaluation — a spawned worker silently +# produced `null` instead of its result, with its frames deliberately +# undrained. Measured before the fix: null on 3 of 3 runs; 400000 after. +# The counts are sized so main is still yielding while the workers run. +define _susp_busy(n) as: + local acc is 0 + local i is 0 + loop while i < 400000: + acc is acc + (abs of (0 - 1)) + i is i + 1 + return acc + +_susp_w1 is spawn of [_susp_busy, 1] +_susp_w2 is spawn of [_susp_busy, 2] +_susp_m is 0 +loop while _susp_m < 100000: + task_yield of null + _susp_m is _susp_m + 1 +assert_eq of [thread_join of _susp_w1, 400000, + "task_yield does not suspend another OS thread (#739)"] +assert_eq of [thread_join of _susp_w2, 400000, + "second worker also unaffected by a foreign suspend request"] + test_summary of null From 1a5add1f74cd8daec995e233f1e18cae5d9a6507 Mon Sep 17 00:00:00 2001 From: InauguralPhysicist Date: Wed, 29 Jul 2026 02:29:45 -0500 Subject: [PATCH 2/2] fix(runtime): four more process-globals that contradict the multi-state contract (#739 item 4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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, 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 the value was read "once per thread", which was never true — once per PROCESS. - g_sandbox_active / _bytes_used / _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; an unclosed stream also lost its buffered tail at exit instead of being flushed (verified: 24 bytes now reach disk where they previously did not). The cleanup is carved out under !EIGENSCRIPT_FREESTANDING with the stream_* builtins themselves — the freestanding symbol gate caught fclose leaving the allowlist. - g_db_conn moved onto EigsState, closed by a new ext_db_state_destroy called from eigs_state_destroy — the shape ext_http_state_destroy already had. One worker's db_connect used to replace the connection another worker was querying through, and nothing ever closed it. NOT BUILT OR RUN: this box has no libpq (no headers, no library), so ext_db.c and state.c with EXT_DB=1 are syntax-checked against a local stub header only. Worth a careful look in review. g_model is deliberately left process-wide. Its allocations are reachable from a global at exit, so they are still-reachable rather than leaked — LSan does not report them. The genuine defect would be cross-state clobber, and per-state transformer weights multiply memory by the number of states; that is a design decision about model isolation, not a mechanical scope move. Same for g_model_age / g_training_samples. Gates: release 3274/3274; asan+ubsan 3278/3278, leak tally 0; TSan 11/11 including its seeded-race self-validation (17 warnings — the gate is live); freestanding stage 1+2 OK (after the carve-out); jit-smoke; sandbox + task + HTTP suites pass. Refs #739 Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 33 +++++++++++++++++++++++++++++++++ docs/BUILTINS.md | 2 +- docs/EMBEDDING.md | 11 +++++++++++ src/builtins.c | 4 +++- src/eigenscript.h | 29 +++++++++++++++++++++++++++++ src/ext_db.c | 9 ++++++++- src/ext_db_internal.h | 8 +++++++- src/jit.c | 6 ------ src/jit.h | 1 - src/state.c | 14 ++++++++++++++ src/vm.c | 24 ++++++++++++------------ src/vm.h | 14 ++++++++------ 12 files changed, 126 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c43f0de..494f9903 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -171,6 +171,39 @@ 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 diff --git a/docs/BUILTINS.md b/docs/BUILTINS.md index 5d160be7..b7b1aafa 100644 --- a/docs/BUILTINS.md +++ b/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 | diff --git a/docs/EMBEDDING.md b/docs/EMBEDDING.md index bc7a1645..7a71b9fa 100644 --- a/docs/EMBEDDING.md +++ b/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 diff --git a/src/builtins.c b/src/builtins.c index c9fd9f6d..8a54eccd 100644 --- a/src/builtins.c +++ b/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) { diff --git a/src/eigenscript.h b/src/eigenscript.h index fce38d0e..cae083cf 100644 --- a/src/eigenscript.h +++ b/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 { @@ -733,6 +740,23 @@ struct EigsThread { * 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; }; @@ -817,6 +841,11 @@ extern __thread EigsThread *eigs_current; #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) diff --git a/src/ext_db.c b/src/ext_db.c index b969636b..86b69830 100644 --- a/src/ext_db.c +++ b/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 diff --git a/src/ext_db_internal.h b/src/ext_db_internal.h index aa92ac9d..86ed22f0 100644 --- a/src/ext_db_internal.h +++ b/src/ext_db_internal.h @@ -9,8 +9,14 @@ #include "eigenscript.h" #include -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 diff --git a/src/jit.c b/src/jit.c index 35f953e8..8b5885a2 100644 --- a/src/jit.c +++ b/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 diff --git a/src/jit.h b/src/jit.h index 0132daab..91c8ed7c 100644 --- a/src/jit.h +++ b/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 diff --git a/src/state.c b/src/state.c index 34863ddd..242e11f1 100644 --- a/src/state.c +++ b/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 diff --git a/src/vm.c b/src/vm.c index b20c42d2..7f0171a5 100644 --- a/src/vm.c +++ b/src/vm.c @@ -704,11 +704,10 @@ static Value *make_iter_state(Value *iterable) { * g_vm fields without indirection through helper functions. */ #include -/* 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; diff --git a/src/vm.h b/src/vm.h index 214169d5..bdcf88e5 100644 --- a/src/vm.h +++ b/src/vm.h @@ -604,8 +604,8 @@ int chunk_verify(EigsChunk *chunk); /* Compiler */ EigsChunk *compile_ast(ASTNode *ast, Env *env, const char *src); -/* Sandbox loop-iteration cap (0 = default 100M). Set by builtin_sandbox_run. */ -extern int g_sandbox_loop_max; +/* Sandbox loop-iteration cap (0 = default 100M). Set by builtin_sandbox_run. + * Per-thread bridge macro in eigenscript.h (#739). */ /* Async abort seam: embedder-registered flag polled on every loop * back-edge — interpreter AND JIT (#410) — see the definition in vm.c and @@ -617,10 +617,12 @@ extern volatile int g_vm_abort_never; /* #292: sandbox allocation budget. Set by builtin_sandbox_run; size-controlled * allocators call sandbox_charge() before allocating. Returns 1 if the charge * fits (and commits it), 0 if it would exceed the budget (after raising a - * catchable runtime_error). No-op when g_sandbox_active is 0. */ -extern int g_sandbox_active; -extern size_t g_sandbox_bytes_used; -extern size_t g_sandbox_byte_max; + * catchable runtime_error). No-op when g_sandbox_active is 0. + * + * g_sandbox_active / _bytes_used / _byte_max are per-thread bridge macros in + * eigenscript.h (#739): as process globals, two states running concurrently + * (ext_http, one state per connection) shared one budget and one active + * flag. */ int sandbox_charge(size_t bytes); /* VM execution */