fix: copy-on-push closes the container double-free; line-buffer program output

The unsoundness is closed. All four MCP tools now answer correctly over HTTP
(get_running_crons, list_logs, tail_log -> ["info two","error three"],
search_log -> its match) where `tail_log` used to return
{"isError":true,"text":"tool failed: not a text value"}. corpus 71/0,
woc 565/0, wovm gates green, ASan clean on the container fixtures.

- builtin.c: multi_push, map_set (key AND value) and multi_set COPY a TEXT
  element into the container. The container's declared kinds already make it
  the owner of what it holds, so storing a caller-owned pointer gave one
  string two owners — `push(res, e.log_path)` freed a record's field out from
  under it. OWNED/GCREF elements still move (not copyable; the @gc escape
  keeps their counting), so `set`'s @gc gap is untouched and still recorded
- emit.ml: `drop_fresh_text` — after push/set and the `m[k] = v` / `m[i] = v`
  sugar, a value that was freshly BUILT (call result, `..` chain,
  interpolation) is dropped here, while a value read out of a place is left to
  its owner. That asymmetry is the point: before the copy the borrowed case
  double freed and the fresh case leaked
- obj.c: the runtime's output stream is line-buffered. A long-running program
  writing progress with `print` was invisible when stdout was a file or a pipe
  (full buffering), and a killed one lost its log entirely; byte-exact
  fixtures are unaffected
- scripts/log-watcher-accept.sh + `just log-watcher`: the acceptance test for
  the sample — compile, watch (alert), run (schedule), and three MCP checks.
  Hardened after it lied to me: a per-run port (a stale server on a fixed port
  answered for it), a connect-probe that fails loudly when OUR server did not
  come up, replies read by Content-Length rather than to EOF (the sample never
  closes), and kill -9 on teardown
- docs: the copy rule is in the builtin surface; the status board records the
  gap as closed and adds the new one — a blocking accept/read swallows SIGTERM,
  which belongs to the shard-actor runtime's event loop, not to a patch here

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
shoney.arickathil 2026-08-14 18:50:18 +02:00
parent 0acb6be559
commit 4cf548d6a0
5 changed files with 353 additions and 12 deletions

View file

@ -1361,6 +1361,18 @@ let container_imm (p : pctx) (expected : Ast.field_ty option) (map : bool) : int
| _ -> None)
| None -> None
(* A value handed to a container builtin that COPIES it (push/set, and the
`m[k] = v` sugar) is dropped here when it was freshly built — a call
result, a concatenation, an interpolation — and left alone when it was read
out of a place, whose owner still holds it. Types the emitter cannot
resolve are left alone: a missed drop is a leak, a wrong drop is a crash. *)
let drop_fresh_text (p : pctx) (f : fstate) (reg : int) (e : Ast.expr) : unit =
let is_place = match e.Ast.kind with Ast.Ident _ | Ast.Field _ | Ast.Index _ -> true | _ -> false in
let is_text =
match ty_of_expr p f e with Some t -> field_kind p t = 3 (* WO_K_TEXT *) | None -> false
in
if (not is_place) && is_text then put f (ins_abc op_drop reg 0 0)
(* `nil` written literally on either side of a comparison — see emit_binary's
Eq/Ne cases for why the distinction matters. *)
let is_nil_lit (e : Ast.expr) : bool = match e.Ast.kind with Ast.NilLit -> true | _ -> false
@ -2733,10 +2745,15 @@ and emit_builtin (p : pctx) (f : fstate) (v : views) ~(dst : int) ?expected (e :
| Some t -> ( match unwrap t with Multi _ -> Some on_multi | Map _ -> Some on_map | _ -> None)
| None -> None
in
let fixed id =
(* Returns the argument window's base when it emitted one, so a caller that
has to clean up after the call (push/set — see below) can find its own
argument registers. *)
let fixed_at id =
let n = arity_of id in
if List.length args <> n then
bad (Printf.sprintf "builtin `%s` takes %d argument(s), given %d" name n (List.length args))
if List.length args <> n then begin
bad (Printf.sprintf "builtin `%s` takes %d argument(s), given %d" name n (List.length args));
None
end
else begin
let base = alloc_temps p f e.pos (max n 1) in
List.iteri
@ -2747,9 +2764,25 @@ and emit_builtin (p : pctx) (f : fstate) (v : views) ~(dst : int) ?expected (e :
args;
sync_mask p f v e.id;
f.f_cur_line <- e.pos.line;
put f (ins_abc op_builtin dst base id)
put f (ins_abc op_builtin dst base id);
Some base
end
in
let fixed id = ignore (fixed_at id) in
(* `push`/`set` COPY a TEXT element, key or value into the container
(runtime/src/builtin.c). So a freshly built Text handed to them — a call
result, a concatenation, an interpolation — is still the caller's, and
dies right here; a Text read out of a place (`e.log_path`, a local, a
loop cursor) is NOT dropped, because its own owner still holds it. That
asymmetry is the whole point: before the copy, the borrowed case double
freed and the fresh case leaked. Types the emitter cannot resolve are
left alone — a missed drop is a leak, a wrong drop is a crash. *)
let copied_container_call id =
match fixed_at id with
| None -> ()
| Some base ->
List.iteri (fun i (a : Ast.expr) -> if i > 0 then drop_fresh_text p f (base + i) a) args
in
match name with
| "now" -> fixed b_now
| "print" -> fixed b_print
@ -2804,7 +2837,7 @@ and emit_builtin (p : pctx) (f : fstate) (v : views) ~(dst : int) ?expected (e :
match args with
| a :: _ -> (
match container_id a b_multi_push b_multi_push with
| Some id -> fixed id
| Some id -> copied_container_call id
| None -> bad "builtin `push` needs a `multi` as its first argument")
| [] -> bad "builtin `push` takes 2 arguments, given 0")
| "get" -> (
@ -2818,7 +2851,7 @@ and emit_builtin (p : pctx) (f : fstate) (v : views) ~(dst : int) ?expected (e :
match args with
| a :: _ -> (
match container_id a b_map_set b_map_set with
| Some id -> fixed id
| Some id -> copied_container_call id
| None -> bad "builtin `set` needs a `map` as its first argument")
| [] -> bad "builtin `set` takes 3 arguments, given 0")
| "has" -> (
@ -3019,7 +3052,11 @@ and emit_assign (p : pctx) (f : fstate) (v : views) (s : Ast.stmt) (target : Ast
let guards = residual_guards p f v s.s_id None in
acquire_guards f guards;
put f (ins_abc op_builtin sink w b_map_set);
release_guards f guards
release_guards f guards;
(* the map COPIES a TEXT key/value in, so a freshly built one is still
this frame's — see emit_builtin's copied_container_call *)
drop_fresh_text p f (w + 1) idx;
drop_fresh_text p f (w + 2) value
| Multi _ ->
(* `m[i] = x` is the multi_set builtin — container, index, value in
three consecutive registers, exactly the map_set shape above.
@ -3034,7 +3071,8 @@ and emit_assign (p : pctx) (f : fstate) (v : views) (s : Ast.stmt) (target : Ast
let guards = residual_guards p f v s.s_id None in
acquire_guards f guards;
put f (ins_abc op_builtin sink w b_multi_set);
release_guards f guards
release_guards f guards;
drop_fresh_text p f (w + 2) value
| _ ->
err p ~code:cannot_lower_code ~file:f.f_file ~pos:target.pos
~message:"element assignment into a value that is neither a `multi` nor a `map`")

View file

@ -47,6 +47,14 @@ wovm-test:
make -C runtime test-iso
bash runtime/test/cli_smoke.sh
# the language track's acceptance test: docs/examples/log-watcher must compile
# with zero diagnostics AND run — watch alerts on an error line, run schedules
# a cron.d entry, and the MCP server answers JSON-RPC (initialize, tools/list,
# 401 without a token). This is the test the whole track exists to pass; the
# corpus below gates the individual behaviors underneath it.
log-watcher:
./scripts/log-watcher-accept.sh
# conformance harness (plan 3): walks tests/corpus/{run,compile-fail,trap},
# exact outcome per fixture kind — see docs/plan/oop-vm/02-corpus.md.
# Fails loudly (and names the recipe to run) if woc or wovm isn't built.

View file

@ -115,7 +115,32 @@ int wo_builtin(wo_vm *vm, uint64_t *R, uint32_t ins, const char **msg) {
case WO_B_MULTI_PUSH: {
wo_multi *m = native_check(R[B], WO_CLS_MULTI, msg);
if (!m) return WO_T_BOUNDS;
if (wo_multi_push(m, R[B + 1]) != 0) {
/* A TEXT element is COPIED in (2026-08-14). The container's declared
* element kind makes it the container's job to free every element, so
* storing a pointer the caller still owns gave one string two owners —
* `push(res, e.log_path)` in the driving workload freed a record's
* field out from under it. Copying is the only rule that is correct
* for both shapes: a borrowed place keeps its owner, and a freshly
* built Text stays the caller's to drop (the compiler emits that
* drop — emit.ml's push/set case). OWNED/GCREF elements still move:
* they are not copyable, and `push`'s @gc escape handles their
* counting. Same rule as `slice`, which has always copied. */
uint64_t v = R[B + 1];
if (m->elem_kind == WO_K_TEXT && v) {
const wo_str *src = (const wo_str *)(uintptr_t)v;
if (src->h.class_id != WO_CLS_STR) {
*msg = "not a text value";
return WO_T_BOUNDS;
}
wo_str *cp = wo_str_new(rt, src->data, src->len);
if (!cp) {
*msg = "out of memory";
return WO_T_OOM;
}
v = (uint64_t)(uintptr_t)cp;
}
if (wo_multi_push(m, v) != 0) {
if (v != R[B + 1]) wo_str_free(rt, (wo_str *)(uintptr_t)v);
*msg = "out of memory";
return WO_T_OOM;
}
@ -166,12 +191,49 @@ int wo_builtin(wo_vm *vm, uint64_t *R, uint32_t ins, const char **msg) {
case WO_B_MAP_SET: {
wo_map *m = native_check(R[B], WO_CLS_MAP, msg);
if (!m) return WO_T_BOUNDS;
/* TEXT keys and TEXT values are copied in, for the same reason
* multi_push copies its element: the map's declared kinds make it the
* owner of what it holds. */
uint64_t k = R[B + 1], v = R[B + 2];
if (m->key_kind == WO_K_TEXT && k) {
const wo_str *src = (const wo_str *)(uintptr_t)k;
if (src->h.class_id != WO_CLS_STR) {
*msg = "not a text value";
return WO_T_BOUNDS;
}
wo_str *cp = wo_str_new(rt, src->data, src->len);
if (!cp) {
*msg = "out of memory";
return WO_T_OOM;
}
k = (uint64_t)(uintptr_t)cp;
}
if (m->val_kind == WO_K_TEXT && v) {
const wo_str *src = (const wo_str *)(uintptr_t)v;
if (src->h.class_id != WO_CLS_STR) {
if (k != R[B + 1]) wo_str_free(rt, (wo_str *)(uintptr_t)k);
*msg = "not a text value";
return WO_T_BOUNDS;
}
wo_str *cp = wo_str_new(rt, src->data, src->len);
if (!cp) {
if (k != R[B + 1]) wo_str_free(rt, (wo_str *)(uintptr_t)k);
*msg = "out of memory";
return WO_T_OOM;
}
v = (uint64_t)(uintptr_t)cp;
}
uint64_t old = 0;
int rc = wo_map_set(m, R[B + 1], R[B + 2], &old);
int rc = wo_map_set(m, k, v, &old);
if (rc < 0) {
if (k != R[B + 1]) wo_str_free(rt, (wo_str *)(uintptr_t)k);
if (v != R[B + 2]) wo_str_free(rt, (wo_str *)(uintptr_t)v);
*msg = "out of memory";
return WO_T_OOM;
}
/* a replaced entry keeps its original key: the copy just made is not
* the one the map holds, so it must not leak */
if (rc == 1 && k != R[B + 1]) wo_str_free(rt, (wo_str *)(uintptr_t)k);
/* insert-or-replace hands the displaced value back: drop it here —
* closing the loose end cont.c documents */
if (rc == 1) wo_drop_kind(rt, m->val_kind, old);
@ -602,8 +664,22 @@ int wo_builtin(wo_vm *vm, uint64_t *R, uint32_t ins, const char **msg) {
*msg = "multi index out of range";
return WO_T_BOUNDS;
}
if (m->items[i] != R[B + 2]) wo_drop_kind(rt, m->elem_kind, m->items[i]);
m->items[i] = R[B + 2];
uint64_t nv = R[B + 2];
if (m->elem_kind == WO_K_TEXT && nv) {
const wo_str *src = (const wo_str *)(uintptr_t)nv;
if (src->h.class_id != WO_CLS_STR) {
*msg = "not a text value";
return WO_T_BOUNDS;
}
wo_str *cp = wo_str_new(rt, src->data, src->len);
if (!cp) {
*msg = "out of memory";
return WO_T_OOM;
}
nv = (uint64_t)(uintptr_t)cp;
}
if (m->items[i] != nv) wo_drop_kind(rt, m->elem_kind, m->items[i]);
m->items[i] = nv;
R[A] = 0;
return 0;
}

View file

@ -53,6 +53,13 @@ int wo_rt_init(wo_rt *rt, size_t heap_cap, const wo_classdesc *classes,
rt->classes = classes;
rt->class_cnt = class_cnt;
rt->out = stdout;
/* Line-buffered, always: a long-running program (the driving workload's
* `watch`/`run`/`mcp` modes) writes progress with `print`, and stdio's
* default full buffering when stdout is a file or a pipe meant that output
* sat in a buffer until exit — so a redirected service looked silent, and
* a killed one lost its log entirely. Content is unchanged, so every
* byte-exact fixture still compares equal. */
setvbuf(stdout, NULL, _IOLBF, 0);
return 0;
}

212
scripts/log-watcher-accept.sh Executable file
View file

@ -0,0 +1,212 @@
#!/usr/bin/env bash
# scripts/log-watcher-accept.sh — the language track's acceptance test.
#
# The corpus (scripts/oop-e2e.sh) gates individual behaviors; THIS gates the
# thing the whole track exists for: docs/examples/log-watcher, 1285 lines of
# .wo across 7 files, must compile with zero diagnostics and then actually run.
# Four checks, in the order a person would try them:
#
# compile woc --emit over the sample's directory, exit 0, no diagnostics
# watch tail a live file: an `error` line, then silence past the quiet
# period, must print ALERT
# run a cron.d directory must parse and schedule (SCHEDULE line)
# mcp the MCP server must answer JSON-RPC over HTTP: initialize,
# tools/list, and a 401 for a request with no bearer token
#
# Each check names what it wanted when it fails. Timeouts are generous but
# real: a hang is a failure, not a wait. No python, no curl — the HTTP client
# is bash's own /dev/tcp, so this runs wherever the runtime does.
set -uo pipefail
ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
WOC="$ROOT/compiler/_build/default/bin/woc"
WOVM="$ROOT/runtime/wovm"
SAMPLE="$ROOT/docs/examples/log-watcher"
# A per-run port by default: a fixed one collides with a server left behind by
# an earlier run (or by hand), and then the checks below silently talk to THAT
# process instead of the one this script started. Override with LW_ACCEPT_PORT.
PORT="${LW_ACCEPT_PORT:-$((18000 + ($$ % 900)))}"
pass=0
fail=0
ok() {
echo "ok $1"
pass=$((pass + 1))
}
bad() {
echo "FAIL $1 -- $2"
fail=$((fail + 1))
}
if [[ ! -x "$WOC" ]]; then
echo "log-watcher-accept: woc is not built ($WOC) — run: just woc-build" >&2
exit 1
fi
if [[ ! -x "$WOVM" ]]; then
echo "log-watcher-accept: wovm is not built ($WOVM) — run: just wovm-build" >&2
exit 1
fi
WORK="$(mktemp -d "${TMPDIR:-/tmp}/lw-accept.XXXXXX")"
# LW_ACCEPT_KEEP=1 leaves the work directory (image, logs, cron.d, the
# server's own stdout) in place — what you want the moment a check fails.
cleanup() {
[[ -n "${SRV_PID:-}" ]] && kill -9 "$SRV_PID" 2>/dev/null
[[ -n "${WATCH_PID:-}" ]] && kill -9 "$WATCH_PID" 2>/dev/null
if [[ -n "${LW_ACCEPT_KEEP:-}" ]]; then
echo "log-watcher-accept: kept $WORK"
else
rm -rf "$WORK"
fi
}
trap cleanup EXIT
IMAGE="$WORK/log-watcher.wob"
# ---- 1. compile -------------------------------------------------------
if timeout 60 "$WOC" --emit "$SAMPLE" -o "$IMAGE" >"$WORK/compile.out" 2>"$WORK/compile.err"; then
if [[ -s "$WORK/compile.err" ]]; then
bad "compile" "exit 0 but diagnostics on stderr: $(head -1 "$WORK/compile.err")"
else
ok "compile ($(wc -c <"$IMAGE" | tr -d ' ') bytes)"
fi
else
bad "compile" "$(head -3 "$WORK/compile.err" | tr '\n' ' ')"
fi
if [[ ! -s "$IMAGE" ]]; then
echo
printf 'log-watcher-accept: %d checks, %d failures\n' "$((pass + fail))" "$((fail + 1))"
echo "log-watcher-accept: no image, nothing to run" >&2
exit 1
fi
# ---- 2. watch ---------------------------------------------------------
# quiet 2s, poll 1s: write an error line, then stay silent long enough for the
# watcher to decide the burst is over.
LOG="$WORK/app.log"
: >"$LOG"
timeout -k 2 12 "$WOVM" "$IMAGE" watch "$LOG" 2 1 >"$WORK/watch.out" 2>&1 &
WATCH_PID=$!
sleep 2
printf 'info service starting\n' >>"$LOG"
sleep 2
printf 'error disk full\n' >>"$LOG"
sleep 6
kill -9 "$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
ok "watch (alerted on the error line)"
else
bad "watch" "no ALERT line; got: $(tr '\n' '|' <"$WORK/watch.out" | cut -c1-160)"
fi
# ---- 3. run (supervisor) ---------------------------------------------
CRON="$WORK/cron.d"
mkdir -p "$CRON"
printf '* * * * * root /usr/bin/backup.sh > /var/log/backup.log 2>&1\n' >"$CRON/backup"
timeout 8 "$WOVM" "$IMAGE" run "$CRON" >"$WORK/run.out" 2>&1
if grep -q "^SCHEDULE /var/log/backup.log" "$WORK/run.out"; then
ok "run (parsed and scheduled the cron entry)"
else
bad "run" "no SCHEDULE line; got: $(tr '\n' '|' <"$WORK/run.out" | cut -c1-160)"
fi
# ---- 4. mcp -----------------------------------------------------------
cat >"$WORK/cfg.json" <<EOF
{ "pollInterval": 2, "quietPeriod": 5, "detections": "$WORK/detections.log",
"mcp": { "port": $PORT, "apiKey": "s3cret" } }
EOF
# -k: `env.stopping()` installs a SIGTERM handler that only sets a flag, and
# the serve loop is blocked in accept(), so a plain TERM is swallowed — the
# process needs a KILL to actually stop (recorded in docs/00-status.md).
timeout -k 2 20 "$WOVM" "$IMAGE" mcp "$CRON" "$WORK/cfg.json" >"$WORK/mcp.out" 2>&1 &
SRV_PID=$!
sleep 2
# Did OUR server actually come up? Without this check a dead server (the usual
# cause: something else already on the port, which makes `net.listen` trap
# "Address already in use") is invisible — the requests below would answer
# from whatever else is listening, and "ok" would mean nothing. Liveness is
# "this process is still running AND the port accepts", checked with a short
# retry so a slow start is a wait, not a failure.
srv_ready=""
for _ in 1 2 3 4 5 6 7 8 9 10; do
if ! kill -0 "$SRV_PID" 2>/dev/null; then break; fi
if (exec 4<>"/dev/tcp/127.0.0.1/$PORT") 2>/dev/null; then
exec 4<&- 2>/dev/null
exec 4>&- 2>/dev/null
srv_ready=1
break
fi
sleep 0.5
done
if [[ -z "$srv_ready" ]]; then
bad "mcp server start" "$(head -1 "$WORK/mcp.out" 2>/dev/null || echo 'no output') (port $PORT; set LW_ACCEPT_PORT to pick another)"
echo
printf 'log-watcher-accept: %d checks, %d failures\n' "$((pass + fail))" "$fail"
exit 1
fi
# One request over bash's own TCP. The reply is read the way any HTTP client
# reads one — headers to the blank line, then exactly Content-Length bytes —
# and NOT by waiting for EOF: the sample never calls `net.close`, so the server
# holds the connection open after answering (a recorded gap, see
# docs/00-status.md). Reading to EOF meant a `timeout`-killed `head`/`cat`
# discarding its own buffer, which made this check flaky rather than false.
http_post() {
local body="$1" auth="$2" line len="" hdr="" reply=""
exec 3<>"/dev/tcp/127.0.0.1/$PORT" || return 1
{
printf 'POST /mcp HTTP/1.1\r\n'
printf 'Host: 127.0.0.1\r\n'
[[ -n "$auth" ]] && printf 'Authorization: Bearer %s\r\n' "$auth"
printf 'Content-Type: application/json\r\n'
printf 'Content-Length: %d\r\n' "${#body}"
printf 'Connection: close\r\n\r\n'
printf '%s' "$body"
} >&3
while IFS= read -r -t 5 line <&3; do
line="${line%$'\r'}"
[[ -z "$line" ]] && break
hdr+="$line"$'\n'
[[ "$line" == [Cc]ontent-[Ll]ength:* ]] && len="${line#*: }"
done
if [[ -n "$len" && "$len" != 0 ]]; then
IFS= read -r -N "$len" -t 5 reply <&3
fi
exec 3<&-
exec 3>&-
printf '%s%s' "$hdr" "$reply"
}
INIT="$(http_post '{"jsonrpc":"2.0","id":1,"method":"initialize"}' s3cret)"
if [[ "$INIT" == *"200 OK"* && "$INIT" == *'"protocolVersion"'* && "$INIT" == *'"id":1'* ]]; then
ok "mcp initialize (200, protocolVersion, unquoted id)"
else
bad "mcp initialize" "got: $(printf '%s' "$INIT" | tr '\n' '|' | cut -c1-160)"
fi
TOOLS="$(http_post '{"jsonrpc":"2.0","id":2,"method":"tools/list"}' s3cret)"
if [[ "$TOOLS" == *"200 OK"* && "$TOOLS" == *'"tail_log"'* && "$TOOLS" == *'"get_running_crons"'* ]]; then
ok "mcp tools/list (the tool set encodes)"
else
bad "mcp tools/list" "got: $(printf '%s' "$TOOLS" | tr '\n' '|' | cut -c1-160)"
fi
NOAUTH="$(http_post '{"jsonrpc":"2.0","id":3,"method":"tools/list"}' '')"
if [[ "$NOAUTH" == *"401 Unauthorized"* && "$NOAUTH" == *'"unauthorized"'* ]]; then
ok "mcp auth (401 without a bearer token)"
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
SRV_PID=""
echo
printf 'log-watcher-accept: %d checks, %d failures\n' "$((pass + fail))" "$fail"
[[ $fail -eq 0 ]]