From 22910e39741fa9624a347744e333ab0e88035e97 Mon Sep 17 00:00:00 2001 From: "shoney.arickathil" Date: Fri, 14 Aug 2026 23:58:21 +0200 Subject: [PATCH] fix: a stopping program stops (executable plan, Task 4) - blocking stdlib calls that PARK (net.accept, socket read/write, time.sleep, a child wait) no longer restart the syscall when the stop flag is set on an interruption: a server sitting in accept ignored SIGTERM and only `kill -9` ended it - a stop is NOT a trap -- builtin.h's WO_SYS_STOPPED carries no error record and no catch handler sees it (`try` must not swallow SIGTERM); the VM unwinds the whole stack through the same drop machinery an uncaught trap uses, so nothing leaks on the way out - wo_vm_call gained a third outcome (1 = stopped); the CLI maps it to the status the program's own `return 0` would have given, and a regular-file read keeps its plain EINTR retry -- it does not park - an ASSIGNMENT was not an ownership boundary: `api_key = j.mcp.apiKey` moved the field pointer into the local, so the local aliased the record and the first unwind freed the same string twice (SIGSEGV in class_free). `let` copied a Text place, assignment now does too -- the same double free was latent on the normal exit path, hidden by the order the compiler happens to emit drops in - log-watcher-accept is 7 checks: the seventh is the stop itself, with the hard kill demoted to a fallback whose use is the failure - measured under ASan: mcp parked, mcp after traffic, watch and run all exit rc 0 with zero leaks; SIGINT behaves as SIGTERM - gates: oop-accept ALL CRITERIA MET, oop-e2e 71/0, woc-test 565/0, wovm-test green, log-watcher 7/0 Co-Authored-By: Claude Opus 5 (1M context) --- compiler/src/emit.ml | 11 ++++++- docs/00-status.md | 12 +++++-- .../2026-08-14-logwatcher-executable.md | 33 +++++++++++++++---- runtime/src/builtin.h | 8 +++++ runtime/src/main.c | 10 ++++-- runtime/src/sysio.c | 33 +++++++++++++++---- runtime/src/vm.c | 11 +++++++ runtime/src/vm.h | 5 ++- scripts/log-watcher-accept.sh | 24 ++++++++++++-- 9 files changed, 124 insertions(+), 23 deletions(-) diff --git a/compiler/src/emit.ml b/compiler/src/emit.ml index d4e8f5a..daead3c 100644 --- a/compiler/src/emit.ml +++ b/compiler/src/emit.ml @@ -3135,6 +3135,12 @@ and emit_assign (p : pctx) (f : fstate) (v : views) (s : Ast.stmt) (target : Ast stored *) let t = alloc_temp p f value.pos in emit_expr p f v ~dst:t ~expected:ty value; + (* an assignment is an ownership boundary exactly as a `let` is — + `api_key = j.mcp.apiKey` made the local ALIAS the record's field, + so dropping the record left the local dangling and its own drop + freed the string a second time. Copy before the old value dies: + the source can live inside what is about to be dropped. *) + copy_place_text p f t value; f.f_cur_line <- s.s_pos.line; if overwrite then begin put f (ins_abc op_drop r 0 0); @@ -3146,7 +3152,10 @@ and emit_assign (p : pctx) (f : fstate) (v : views) (s : Ast.stmt) (target : Ast end; put f (ins_abc op_move r t 0) end - else emit_expr p f v ~dst:r ~expected:ty value; + else begin + emit_expr p f v ~dst:r ~expected:ty value; + copy_place_text p f r value + end; (* a whole-local target cannot be an unprovable alias of anything (relate answers Overlap or Disjoint for a place with no projections), so this normally finds nothing; consumed anyway so diff --git a/docs/00-status.md b/docs/00-status.md index 1acd0da..ed35100 100644 --- a/docs/00-status.md +++ b/docs/00-status.md @@ -50,9 +50,15 @@ running", and every item below came from a measurement on the sample itself: built, after the entry returns and after a trap alike. **All three modes now report ZERO leaks under ASan** — `watch`, `run`, and the full MCP mix — which is the clean baseline item 6's soak needs to read against. -4. **A stopping program does not stop** — `env.stopping()` sets a flag, but - `net.accept`/`net.read` restart on `EINTR`, so a server parked in `accept` - ignores SIGTERM and needs `kill -9`. +4. ~~A stopping program does not stop~~ — **done 2026-08-14**. A blocking + call that parks (`net.accept`, socket read/write, `time.sleep`, a child + wait) now ends the program when it is interrupted with the stop flag set, + instead of restarting the syscall. A stop is not a trap: `try` cannot + swallow it, and the stack unwinds through the same drop machinery, so the + exit is clean and leak-free in every mode. It also uncovered a real + double-free: an **assignment** of a Text place was a move, not a copy, so + `api_key = j.mcp.apiKey` aliased the record — `let` copied, assignment now + does too. `just log-watcher` is 7 checks; the seventh is the stop. 5. **The MCP server never closes an accepted connection** — `net.close` exists and is unused; every request costs a descriptor. 6. **Nothing soaks** — every check is seconds long, which is exactly the window diff --git a/docs/plan/compiler/2026-08-14-logwatcher-executable.md b/docs/plan/compiler/2026-08-14-logwatcher-executable.md index a281369..5efaf63 100644 --- a/docs/plan/compiler/2026-08-14-logwatcher-executable.md +++ b/docs/plan/compiler/2026-08-14-logwatcher-executable.md @@ -166,7 +166,7 @@ before the heap is torn down. CRITERIA MET, `just oop-e2e` 71/0, `just wovm-test` green, `just log-watcher` 6/0. -### Task 4: A stopping program must actually stop +### Task 4 ✅: A stopping program must actually stop **Concept & reason:** `env.stopping()` installs SIGTERM/SIGINT handlers that set a flag, and `net.accept`/`net.read` retry on `EINTR` — so a server parked in @@ -182,11 +182,32 @@ would put a trap in the middle of every accept loop the language will ever write, and the shard-actor runtime (iteration 8) replaces these blocking calls with an event loop anyway. -- [ ] Failing measurement: `mcp` mode ignores SIGTERM and needs `kill -9`. -- [ ] Blocking stdlib calls observe the stop flag on interruption; the process - exits cleanly, flushing output. -- [ ] `just log-watcher` no longer needs `kill -9` in teardown, and the script's - hard-kill fallback becomes belt-and-braces rather than the mechanism. +- [x] Failing measurement: `mcp` mode ignored SIGTERM and needed `kill -9`. +- [x] The calls that genuinely PARK — `net.accept`, a socket read/write, + `time.sleep`, a child wait — no longer restart the syscall when the stop + flag is set on an interruption. They hand back `WO_SYS_STOPPED` + (builtin.h), which is **not** a trap code: no error record, no catch + handler sees it (a `try` must not be able to swallow SIGTERM), and the + VM unwinds the whole stack through the same drop machinery an uncaught + trap uses, so every live value is still released. `wo_vm_call` gained a + third outcome (1 = stopped) and the CLI maps it to the status the + program's own `return 0` would have produced. A regular-file read keeps + its plain retry — it does not park. +- [x] **Found and fixed while measuring: an assignment was not an ownership + boundary.** `api_key = j.mcp.apiKey` MOVED the field pointer into the + local, so the local aliased the record; the first stop unwind dropped + the record and then the alias, freeing the same string twice (a SIGSEGV + in `class_free`). `let` copied a Text place, assignment did not — it + does now. The same double free was latent on the normal exit path, + hidden only by the drop ORDER the compiler happens to emit. +- [x] `just log-watcher` is **7 checks** now: the seventh is the stop itself — + SIGTERM sent to a server parked in `accept` with no traffic coming, with + the hard kill demoted to a fallback whose use is the failure. +- [x] Re-measured, all under ASan: `mcp` stopped while parked (rc 0, zero + leaks), `mcp` stopped after serving traffic (rc 0, zero leaks), `watch` + and `run` stopped mid-poll (rc 0, zero leaks), and SIGINT behaves as + SIGTERM. Gates: `just oop-accept` ALL CRITERIA MET, `oop-e2e` 71/0, + `woc-test` 565/0, `wovm-test` green, `just log-watcher` 7/0. ### Task 5: The MCP server must close what it accepts diff --git a/runtime/src/builtin.h b/runtime/src/builtin.h index 7153ea0..88c40ab 100644 --- a/runtime/src/builtin.h +++ b/runtime/src/builtin.h @@ -7,6 +7,14 @@ #include "vm.h" +/* Not a trap code: the stop flag (SIGTERM/SIGINT, armed by `env.stopping`) + * was set when a BLOCKING call was interrupted, so the call does not restart + * the syscall — it hands this back and the VM ends the program with it. A + * service parked in `accept` otherwise never observes the flag and only + * `kill -9` ends it. Negative so it cannot collide with a WO_T_* code, and + * deliberately NOT catchable: `try` must not be able to swallow a stop. */ +#define WO_SYS_STOPPED (-2) + int wo_builtin(wo_vm *vm, uint64_t *R, uint32_t ins, const char **msg); /* The systems stdlib's OS half (runtime/src/sysio.c): same contract as diff --git a/runtime/src/main.c b/runtime/src/main.c index 3cf2e19..00746ea 100644 --- a/runtime/src/main.c +++ b/runtime/src/main.c @@ -189,12 +189,16 @@ int main(int argc, char **argv) { wo_err terr; int rc = wo_vm_call(&VM, mod.entry, entry_argc == 1 ? &argv_val : NULL, entry_argc, &ret, &terr); - if (rc != 0) + if (rc < 0) fprintf(stderr, "trap %u in %s at line %u: %s\n", (unsigned)terr.code, terr.method, (unsigned)terr.line, terr.msg); /* the entry's return value IS the exit code (docs/plan/oop-vm/ - * 08-builtin-surface.md's "Program entry"): 0..255, a trap is 1 */ - int exit_code = rc == 0 ? (int)((uint64_t)ret & 0xFF) : 1; + * 08-builtin-surface.md's "Program entry"): 0..255, a trap is 1. A stop + * (rc == 1) is not a failure and not a trap — SIGTERM landing in a + * blocking call is the operator asking for the shutdown the program + * would have taken at its own next `env.stopping()` check, so it exits + * with the status that check's `return 0` would have produced. */ + int exit_code = rc == 0 ? (int)((uint64_t)ret & 0xFF) : (rc > 0 ? 0 : 1); /* the entry only BORROWS its arguments -- a parameter is never a `take`, * and the drop tables never drop one -- so the runtime that built the * container is the one that releases it, elements included. Without this diff --git a/runtime/src/sysio.c b/runtime/src/sysio.c index 745cc0e..d120bf3 100644 --- a/runtime/src/sysio.c +++ b/runtime/src/sysio.c @@ -82,6 +82,13 @@ static void on_stop(int sig) { stop_flag = 1; } +/* An interrupted blocking call asks this before restarting the syscall: a + * set flag means the program was told to stop, and the calls below stop + * instead of restarting (builtin.h's WO_SYS_STOPPED). Only the calls that + * genuinely PARK consult it — accept, a socket read/write, sleep and a child + * wait. A regular-file read is not one of them and keeps its plain retry. */ +static int stop_pending(void) { return stop_flag != 0; } + static void install_stop_handlers(void) { if (stop_installed) return; stop_installed = 1; @@ -262,7 +269,10 @@ int wo_builtin_sys(wo_vm *vm, uint64_t *R, uint32_t ins, const char **msg) { int64_t ms = (int64_t)R[B]; if (ms > 0) { struct timespec ts = {ms / 1000, (ms % 1000) * 1000000L}, rem; - while (nanosleep(&ts, &rem) != 0 && errno == EINTR) ts = rem; + while (nanosleep(&ts, &rem) != 0 && errno == EINTR) { + if (stop_pending()) return WO_SYS_STOPPED; + ts = rem; + } } R[A] = 0; return 0; @@ -358,9 +368,11 @@ int wo_builtin_sys(wo_vm *vm, uint64_t *R, uint32_t ins, const char **msg) { } case WO_B_NET_ACCEPT: { int fd; - do { + for (;;) { fd = accept((int)R[B], NULL, NULL); - } while (fd < 0 && errno == EINTR); + if (fd >= 0 || errno != EINTR) break; + if (stop_pending()) return WO_SYS_STOPPED; + } if (fd < 0) { *msg = strerror(errno); return WO_T_IO; @@ -378,9 +390,14 @@ int wo_builtin_sys(wo_vm *vm, uint64_t *R, uint32_t ins, const char **msg) { return WO_T_OOM; } ssize_t n; - do { + for (;;) { n = read((int)R[B], s->data, (size_t)max); - } while (n < 0 && errno == EINTR); + if (n >= 0 || errno != EINTR) break; + if (stop_pending()) { + wo_str_free(rt, s); + return WO_SYS_STOPPED; + } + } if (n < 0) { wo_str_free(rt, s); *msg = strerror(errno); @@ -411,7 +428,10 @@ int wo_builtin_sys(wo_vm *vm, uint64_t *R, uint32_t ins, const char **msg) { while (at < body->len) { ssize_t n = write((int)R[B], body->data + at, body->len - at); if (n < 0) { - if (errno == EINTR) continue; + if (errno == EINTR) { + if (stop_pending()) return WO_SYS_STOPPED; + continue; + } *msg = strerror(errno); return WO_T_IO; } @@ -497,6 +517,7 @@ int wo_builtin_sys(wo_vm *vm, uint64_t *R, uint32_t ins, const char **msg) { close(ep[0]); int status = 0; while (waitpid(pid, &status, 0) < 0 && errno == EINTR) { + if (stop_pending()) return WO_SYS_STOPPED; } wo_hdr *o = record_of(vm, R[B + 2], 3, msg); if (!o) return R[B + 2] >= vm->mod->class_cnt ? WO_T_BOUNDS : WO_T_OOM; diff --git a/runtime/src/vm.c b/runtime/src/vm.c index b168a36..3f05bf0 100644 --- a/runtime/src/vm.c +++ b/runtime/src/vm.c @@ -474,6 +474,17 @@ dispatch: CASE(BUILTIN) : { const char *bmsg = "builtin failed"; int brc = wo_builtin(vm, R, ins, &bmsg); + /* A stop is not a trap: no error record, no catch handler gets a + * look (`try` must not be able to swallow SIGTERM), and no message. + * The stack is unwound exactly as an uncaught trap unwinds it, so + * every live value is still released on the way out; the CLI turns + * this into the same exit status a clean `return 0` gives. */ + if (brc == WO_SYS_STOPPED) { + vm->frames[vm->depth - 1].pc = pc - 1; + vm->ncatch = 0; + vm_unwind(vm, 0); + return 1; + } if (brc) TRAPF((uint32_t)brc, "%s", bmsg); NEXT(); } diff --git a/runtime/src/vm.h b/runtime/src/vm.h index 097a70e..28a9487 100644 --- a/runtime/src/vm.h +++ b/runtime/src/vm.h @@ -55,7 +55,10 @@ void wo_vm_destroy(wo_vm *vm); /* Call a method with raw argument words. argc must equal the method's * declared arity. 0 = done, *ret filled; -1 = trapped, *err filled and the - * stack fully unwound (depth 0). */ + * stack fully unwound (depth 0); 1 = STOPPED — a blocking stdlib call was + * interrupted with the stop flag set (builtin.h's WO_SYS_STOPPED), the stack + * is unwound the same way, *ret and *err are untouched, and there is nothing + * to report: the program was told to stop and did. */ int wo_vm_call(wo_vm *vm, uint32_t method_idx, const uint64_t *args, uint32_t argc, uint64_t *ret, wo_err *err); diff --git a/scripts/log-watcher-accept.sh b/scripts/log-watcher-accept.sh index bf16e00..c4cbff7 100755 --- a/scripts/log-watcher-accept.sh +++ b/scripts/log-watcher-accept.sh @@ -94,7 +94,7 @@ printf 'info service starting\n' >>"$LOG" sleep 2 printf 'error disk full\n' >>"$LOG" sleep 6 -kill -9 "$WATCH_PID" 2>/dev/null +kill -TERM "$WATCH_PID" 2>/dev/null wait "$WATCH_PID" 2>/dev/null WATCH_PID="" if grep -q "^watching " "$WORK/watch.out" && grep -q "^ALERT .*last entry is error" "$WORK/watch.out"; then @@ -203,8 +203,26 @@ else bad "mcp auth" "got: $(printf '%s' "$NOAUTH" | tr '\n' '|' | cut -c1-160)" fi -kill -9 "$SRV_PID" 2>/dev/null -wait "$SRV_PID" 2>/dev/null +# The stop check (executable plan, Task 4): the server is parked in +# `net.accept` with no traffic coming, and TERM alone has to end it. Before +# the runtime observed the stop flag on an interrupted blocking call this +# needed `kill -9`, which is not a shutdown — no drops, no flush, nothing a +# supervisor or a deploy can rely on. The hard kill below stays as a +# belt-and-braces fallback, and reaching it is the failure. +kill -TERM "$SRV_PID" 2>/dev/null +STOPPED="" +for _ in $(seq 1 40); do + if ! kill -0 "$SRV_PID" 2>/dev/null; then STOPPED=1; break; fi + sleep 0.1 +done +if [[ -n "$STOPPED" ]]; then + wait "$SRV_PID" 2>/dev/null + ok "mcp stop (SIGTERM ends a server parked in accept)" +else + kill -9 "$SRV_PID" 2>/dev/null + wait "$SRV_PID" 2>/dev/null + bad "mcp stop" "still running 4s after SIGTERM; needed kill -9" +fi SRV_PID="" echo