From 3794516096a65d745fd9eec5b90e076c898c651c Mon Sep 17 00:00:00 2001 From: InauguralPhysicist Date: Thu, 9 Jul 2026 04:26:56 -0500 Subject: [PATCH] fix(tasks): unjoined death exit code, arena guard w/o scheduler, suspended-main leak (#493, #510, #483) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three silent-tolerance holes in the #408 cooperative task layer, found by an adversarial language review and closed together (they share the task-builtin and scheduler-teardown seams). #493 — a worker that dies of an uncaught error while nothing task_joins it printed its trace but the process still exited 0: a fire-and-forget worker's death silently greened the run. Now tracked per-task (err_unobserved, set on uncaught death, cleared when a join observes it caught-or-not) and folded into the process exit code — task_kill is a deliberate teardown and never trips it. define die() as: missing id is task_spawn of die print of "x" # main runs to completion... task_yield of null # before: rc=0 after: rc=1 #510 — task_yield/task_recv/task_sleep checked the no-scheduler short-circuit BEFORE the arena guard, so `arena_mark … task_yield … arena_reset` was a no-op (not the documented `value` raise) until some task_spawn had armed a scheduler. The arena guard now runs first, independent of the scheduler. #483 — a fatal exit while main (task 0) is still SUSPENDED (a deadlock, or main blocked on a join/recv that never resolves) leaked ~10 KB: task_sched_thread_free freed main's saved-slice arrays on the "main always ran to completion, slice is empty" assumption, orphaning the module frame's chunk ref (+ nested fn chunks in its constant pool) and the operand stack's value refs. The slice is now torn down ref-correctly, mirroring task_free's worker path. (Workers were already clean via handle_table_drain → task_free.) The one-shot LSan repro read clean — a stale stack slot kept the block reachable, a false negative — and only the in-suite deadlock test made it reproduce; that test is the regression lock. Tests: #510 arena-guard-without-scheduler assertions at the top of test_tasks.eigs (red without the fix); a dedicated exit-contract harness in run_all_tests.sh over four fixtures (unjoined-death rc1, join+catch rc0, killed-unjoined rc0, deadlock rc1 + leak-clean under ASan). Docs: SPEC.md task exit-status rule, DIAGNOSTICS.md exit-code table, CHANGELOG. Release + ASan suites: 2687/2687, leak tally 0. Closes #493 Closes #510 Closes #483 Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 29 +++++++++++++++++++ docs/DIAGNOSTICS.md | 1 + docs/SPEC.md | 8 ++++++ src/builtins.c | 24 +++++++++++----- src/main.c | 6 +++- src/vm.c | 44 +++++++++++++++++++++++++---- src/vm.h | 8 ++++++ tests/run_all_tests.sh | 26 +++++++++++++++++ tests/task_deadlock.eigs | 14 +++++++++ tests/task_exit_join_catch.eigs | 9 ++++++ tests/task_exit_killed.eigs | 9 ++++++ tests/task_exit_unjoined_death.eigs | 9 ++++++ tests/test_tasks.eigs | 31 ++++++++++++++++++++ 13 files changed, 205 insertions(+), 13 deletions(-) create mode 100644 tests/task_deadlock.eigs create mode 100644 tests/task_exit_join_catch.eigs create mode 100644 tests/task_exit_killed.eigs create mode 100644 tests/task_exit_unjoined_death.eigs diff --git a/CHANGELOG.md b/CHANGELOG.md index 8469f7e..3c09443 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,35 @@ All notable changes to EigenScript are documented here. region issued no scheduler yield (needs VM support). ### Changed +- **Cooperative task layer — exit-code and arena-guard fixes (#493, #510).** + Two silent-tolerance holes in the #408 task layer: + - **#493:** a worker that dies of an uncaught error while nothing + `task_join`s it now makes the **process exit non-zero** instead of + silently returning 0 — a fire-and-forget worker's death can no longer + green a run (`task_spawn` without a join hid failures). Joining and + catching still recovers (exit 0); a deliberate `task_kill` is a teardown, + not an uncaught error, and does not fail the process. Documented in + docs/SPEC.md (task layer) and the docs/DIAGNOSTICS.md exit-code table. + - **#510:** the arena guard on the suspending task builtins + (`task_yield` / `task_recv` / `task_sleep`) is now checked **before** the + no-scheduler short-circuit, so `arena_mark … task_yield … arena_reset` + raises `value` even before any `task_spawn` — the "no-op with no tasks" + rule no longer hides the documented "forbidden inside an arena" rule. + +- **#483 — leak of the suspended main slice on a fatal exit.** When the process + ends while task 0 (main) is still **suspended** — a `deadlock`, or main + blocked on a join/recv that never resolves — `task_sched_thread_free` freed + main's saved-slice arrays on the "main always ran to completion, slice is + empty" assumption, which is false here. Main's base module frame owns a + counted chunk ref (the script chunk) and its operand stack owns value refs; + those leaked (the script chunk plus every nested fn chunk in its constant + pool — ~10 KB on the deadlock repro). Now the slice is torn down ref-correctly + (mirroring `task_free`'s worker-slice teardown) before the arrays are freed. + Worker tasks were already clean (reaped by `handle_table_drain` → `task_free`); + only main's slice was missed. Gated by a leak-checked deadlock regression test + — which is also what surfaced this: the original repro read clean under a + one-shot LSan run (a stale stack slot kept the block reachable — a false + negative), and only the in-suite test made it reproduce reliably. - **Silent-tolerance audit, batch 2 — invalid input raises instead of returning a silent wrong value** (#497, #498, #499, #501, #502, #511, #512). Builtins that used to fold bad input to a `null`/`0`/`""`/ diff --git a/docs/DIAGNOSTICS.md b/docs/DIAGNOSTICS.md index aab5102..59314ce 100644 --- a/docs/DIAGNOSTICS.md +++ b/docs/DIAGNOSTICS.md @@ -83,6 +83,7 @@ result dict gains `"error": {kind, message, line}` when `ok` is 0. | File not found | 1 | | Syntax / parse errors | 1 | | Uncaught runtime error or `throw` | 1 | +| Unjoined task dies of an uncaught error (fire-and-forget) | 1 | | Assertion failure | 1 | | Errors caught by `try`/`catch` | 0 | | Warnings only | 0 | diff --git a/docs/SPEC.md b/docs/SPEC.md index 0194c24..5b89f63 100644 --- a/docs/SPEC.md +++ b/docs/SPEC.md @@ -1171,6 +1171,14 @@ When the main program returns, any tasks still running are torn down example two tasks each `task_join`-ing the other — that is a `deadlock` error, reported loudly rather than hanging. +A task that **dies of an uncaught error** prints its stack trace, and if +nothing ever `task_join`s it the **process still exits non-zero** — a +fire-and-forget worker's failure is never silently swallowed into a +success exit. `task_join`-ing the dead task and `catch`-ing its error +recovers normally (exit 0), exactly as for an inline `try`/`catch`. A +task ended deliberately with `task_kill` is a teardown, not an uncaught +error, and does not by itself fail the process. + Tasks communicate through **mailboxes**. `task_send of [id, value]` appends a deep-copied message to task `id`'s FIFO mailbox (share- nothing, like channel sends); `task_recv of null` returns the next diff --git a/src/builtins.c b/src/builtins.c index adf3524..65c5662 100644 --- a/src/builtins.c +++ b/src/builtins.c @@ -4728,12 +4728,16 @@ Value* builtin_task_spawn(Value *arg) { * separate tools (Lua-style "can't yield across a C boundary"). */ Value* builtin_task_yield(Value *arg) { (void)arg; - if (!g_task_sched) return make_null(); /* no scheduler: yield is a no-op */ + /* #510: the arena guard is independent of whether a scheduler exists yet. + * Yielding inside arena_mark…arena_reset is forbidden even before any + * task_spawn — check it BEFORE the no-scheduler short-circuit so the + * "no-op with no tasks" rule cannot hide the "raise inside an arena" rule. */ if (g_arena.active) { rt_error(EK_VALUE, 0, "cannot task_yield inside an arena scope " "(arena_mark…arena_reset)"); return make_null(); } + if (!g_task_sched) return make_null(); /* no scheduler: yield is a no-op */ task_request_yield(); return make_null(); /* placeholder: execution resumes right after this */ } @@ -4750,6 +4754,7 @@ Value* builtin_task_join(Value *arg) { if (t->state == TASK_DONE || t->state == TASK_DEAD) { /* Already finished: deliver its result / error now, no suspend. */ if (t->has_error) { + t->err_unobserved = 0; /* #493: observed by this join (caught or not) */ if (t->error_value) { val_incref(t->error_value); eigs_clear_error_value(); @@ -4794,13 +4799,17 @@ Value* builtin_task_send(Value *arg) { * nested-evaluation restriction as the other suspending builtins. */ Value* builtin_task_recv(Value *arg) { (void)arg; - if (!g_task_sched) return make_null(); /* no tasks: nothing can arrive */ + /* A buffered message is delivered without suspending — arena-safe (and + * mbox helpers are null-safe with no scheduler). Only the BLOCKING path + * suspends, so only that path is arena-forbidden, and (#510) that guard + * fires regardless of whether a scheduler exists yet. */ if (task_mbox_has()) return task_mbox_pop(); if (g_arena.active) { rt_error(EK_VALUE, 0, "cannot task_recv inside an arena scope " "(arena_mark…arena_reset)"); return make_null(); } + if (!g_task_sched) return make_null(); /* no tasks: nothing can arrive */ task_request_recv(); return make_null(); /* placeholder: the scheduler delivers the message on resume */ } @@ -4840,16 +4849,17 @@ Value* builtin_task_alive(Value *arg) { * scheduler (no task ever spawned) → no time to pass, so it is a no-op, like * task_yield. Same arena/nesting restriction as the other suspending builtins. */ Value* builtin_task_sleep(Value *arg) { - if (!g_task_sched) return make_null(); /* no scheduler: nothing to wait for */ - if (!arg || arg->type != VAL_NUM) { - rt_error(EK_TYPE, 0, "task_sleep requires a number of ticks"); - return make_null(); - } + /* #510: arena guard before the no-scheduler short-circuit (see task_yield). */ if (g_arena.active) { rt_error(EK_VALUE, 0, "cannot task_sleep inside an arena scope " "(arena_mark…arena_reset)"); return make_null(); } + if (!g_task_sched) return make_null(); /* no scheduler: nothing to wait for */ + if (!arg || arg->type != VAL_NUM) { + rt_error(EK_TYPE, 0, "task_sleep requires a number of ticks"); + return make_null(); + } task_request_sleep(arg->data.num); return make_null(); /* placeholder: execution resumes when the clock wakes it */ } diff --git a/src/main.c b/src/main.c index 7ac42c6..c2c7bea 100644 --- a/src/main.c +++ b/src/main.c @@ -285,6 +285,9 @@ int main(int argc, char **argv) { if (getenv("EIGS_DUMP_BC")) chunk_disassemble(script_chunk, ""); Value *result = vm_execute(script_chunk, global); if (result) val_decref(result); + /* #493: capture whether any worker died of an uncaught error without ever + * being joined — must be read BEFORE handle_table_drain frees the tasks. */ + int unobserved_task_error = task_any_unobserved_error(); /* Program done: deterministically reap workers and free channels while the * value world is still alive (channels/threads live in the handle table, * not on a GC'd Value, so nothing else reclaims them). */ @@ -295,7 +298,8 @@ int main(int argc, char **argv) { /* `exit of N` requests a specific code (and unwound via g_has_error); it * takes precedence over the generic uncaught-error code. Clear the unwind * flag so the teardown below sees a clean state. */ - int exit_code = g_exit_requested ? g_exit_code : (g_has_error ? 1 : 0); + int exit_code = g_exit_requested ? g_exit_code + : ((g_has_error || unobserved_task_error) ? 1 : 0); if (g_exit_requested) g_has_error = 0; /* An uncaught `throw` leaves its structured payload stashed; release * it so exit is leak-clean. */ diff --git a/src/vm.c b/src/vm.c index 2079afd..5519d7a 100644 --- a/src/vm.c +++ b/src/vm.c @@ -5200,11 +5200,26 @@ void task_sched_thread_free(void) { TaskScheduler *s = sched_get(); if (!s) return; Task *m = &s->main_task; - /* main runs to completion before we get here, so its saved slice is empty; - * free defensively. Drain any messages left in main's mailbox + its result - * (nulled by the trampoline on a clean exit, but be defensive). */ - free(m->saved_stack); - free(m->saved_frames); + /* #483: main is USUALLY run-to-completion here (empty slice). But a fatal + * exit while main is still SUSPENDED — a `deadlock`, or main blocked on a + * join/recv that never resolves — leaves a live saved slice whose counted + * refs would otherwise leak: the base module frame owns a chunk ref (the + * script chunk, see vm_run's frame push), and the operand stack owns value + * refs. Release them, mirroring task_free's worker-slice teardown, before + * freeing the arrays. (owns_env is 0 for the module frame — the global env + * is dropped separately in main.c/eigs_close — so only chunk_decref here.) */ + if (m->saved_stack) { + for (int i = 0; i < m->saved_stack_len; i++) slot_decref(m->saved_stack[i]); + free(m->saved_stack); + } + if (m->saved_frames) { + for (int i = 0; i < m->saved_frame_count; i++) { + CallFrame *f = &m->saved_frames[i]; + if (f->owns_env && f->env) env_decref(f->env); + if (f->chunk) chunk_decref(f->chunk); + } + free(m->saved_frames); + } if (m->mbox) { for (int i = 0; i < m->mbox_count; i++) val_decref(m->mbox[(m->mbox_head + i) % m->mbox_cap]); @@ -5221,6 +5236,19 @@ static Task *sched_lookup(TaskScheduler *s, int id) { return (Task *)handle_lookup(id, HANDLE_TASK); } +/* #493: does any worker still carry an uncaught-error death that no task_join + * ever observed? Scanned once at process exit (before handle_table_drain frees + * the tasks) so a fire-and-forget worker's death makes the process exit + * non-zero instead of silently returning 0. */ +int task_any_unobserved_error(void) { + if (!g_task_sched) return 0; + for (int i = 1; i < HANDLE_TABLE_SIZE; i++) { + Task *t = (Task *)handle_lookup(i, HANDLE_TASK); + if (t && t->err_unobserved) return 1; + } + return 0; +} + static Task *task_current_running(void) { TaskScheduler *s = sched_get(); return s ? sched_lookup(s, s->current) : NULL; @@ -5566,6 +5594,11 @@ static void sched_finish(TaskScheduler *s, Task *t, Value *r) { t->has_error = 1; t->error_value = vm_take_error_value(); /* the {kind,message,line} dict */ g_has_error = 0; + /* #493: a worker that dies of an uncaught error must fail the process + * if nothing ever joins it. Main (task 0) already propagates its own + * error via the trampoline's return, so only mark workers here; a + * later task_join on this task clears the flag. */ + if (t->id != 0) t->err_unobserved = 1; if (r) val_decref(r); t->result = NULL; } else { @@ -5597,6 +5630,7 @@ static void task_apply_join_result(Task *t) { t->join_target = 0; if (!jt) return; if (jt->has_error) { + jt->err_unobserved = 0; /* #493: observed by this join (caught or not) */ /* Re-raise: restore the error payload so the joiner's CHECK_ERROR * catches/propagates it as if the throw happened at the join. */ if (jt->error_value) { diff --git a/src/vm.h b/src/vm.h index 5f46572..d5da70b 100644 --- a/src/vm.h +++ b/src/vm.h @@ -428,6 +428,13 @@ typedef struct Task { Value *result; /* owned, deep-copied on normal end */ int has_error; /* died of an uncaught error */ Value *error_value; /* owned {kind,message,line} dict */ + /* #493: set when a worker (id != 0) dies of an uncaught runtime error and + * NOT yet observed by a task_join. Cleared once a joiner delivers/re-raises + * the error (caught or not). A task killed via task_kill is a deliberate + * teardown, not an uncaught error, so it never sets this. Any task left + * with this at process exit makes the process exit non-zero — a + * fire-and-forget worker's death must not silently green a run. */ + int err_unobserved; } Task; /* Free a Task's held refs and the struct. Safe on any state; called by @@ -444,6 +451,7 @@ void task_sched_on_spawn(int id); /* enqueue a freshly spawned task, arm the 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 */ void task_sched_thread_free(void); /* release the scheduler at thread detach */ +int task_any_unobserved_error(void);/* #493: any worker died of an uncaught error and was never joined? */ /* Inc 2 mailbox interface (builtins.c task_send/task_recv/task_try_recv/task_kill). */ int task_deliver(int tid, Value *msg_owned); /* append msg (ownership taken); 1 sent / 0 dropped-to-dead */ int task_mbox_has(void); /* current task has a queued message? */ diff --git a/tests/run_all_tests.sh b/tests/run_all_tests.sh index c6d5204..f9fb5e5 100755 --- a/tests/run_all_tests.sh +++ b/tests/run_all_tests.sh @@ -2236,6 +2236,32 @@ else FAIL=$((FAIL + 1)) fi +# #493 exit-code contract + #483 deadlock leak lock. These assert PROCESS exit +# behavior (not in-language assertions), so they run as a dedicated harness: an +# unjoined uncaught task death fails the process; joining+catching recovers; a +# deliberate task_kill does not fail; a mutual-join deadlock is loud. Every case +# must also be leak-clean under the ASan build (the exact paths #483 covers) — +# any LeakSanitizer report here fails, stricter than the tolerated global tally. +# args: file expected_rc marker +check_task_exit() { + TOTAL=$((TOTAL + 1)) + local file="$1" want_rc="$2" marker="$3" out rc + out=$(./eigenscript "../tests/$file" &1); rc=$? + if [ "$rc" -eq "$want_rc" ] && echo "$out" | grep -q "$marker" \ + && ! echo "$out" | grep -q "LeakSanitizer"; then + echo " PASS: task exit contract — $file (rc=$rc)" + PASS=$((PASS + 1)) + else + echo " FAIL: task exit contract — $file (rc=$rc, want=$want_rc)" + echo "$out" | grep -iE "LeakSanitizer|error|assert" | head -3 + FAIL=$((FAIL + 1)) + fi +} +check_task_exit task_exit_unjoined_death.eigs 1 "MARK_END" # #493 strict: main completes, rc 1 +check_task_exit task_exit_join_catch.eigs 0 "undefined_name" # #493 caught: rc 0 +check_task_exit task_exit_killed.eigs 0 "MARK_END" # #493 kill: rc 0 +check_task_exit task_deadlock.eigs 1 "deadlock" # #483 leak-clean (main's suspended slice) + #509 loud + # [105] Builtin contract fixes (#312 negative indices, #316 predicate # type-rejection, #317 min/max N-ary reduction) + #314: a directory as the # script path must take the clean cannot-read-file exit, not xmalloc's diff --git a/tests/task_deadlock.eigs b/tests/task_deadlock.eigs new file mode 100644 index 0000000..6fb2edf --- /dev/null +++ b/tests/task_deadlock.eigs @@ -0,0 +1,14 @@ +# #483/#509: a mutual join deadlock is loud (rc 1) and — the #483 regression +# lock — leak-clean at exit under ASan. On deadlock, main is left SUSPENDED in +# the module frame, whose saved slice owns the script chunk + nested fn chunks; +# task_sched_thread_free must tear that slice down ref-correctly (it used to +# free the arrays assuming main's slice was empty, leaking ~10 KB). +define da() as: + return task_join of gb +define db() as: + return task_join of ga +ga is 0 +gb is 0 +ga is task_spawn of da +gb is task_spawn of db +task_join of ga diff --git a/tests/task_exit_join_catch.eigs b/tests/task_exit_join_catch.eigs new file mode 100644 index 0000000..826857b --- /dev/null +++ b/tests/task_exit_join_catch.eigs @@ -0,0 +1,9 @@ +# #493: joining the dead worker and CATCHING its error recovers cleanly (rc 0). +define die() as: + missing +id is task_spawn of die +try: + task_join of id +catch e: + print of e.kind +print of "MARK_END" diff --git a/tests/task_exit_killed.eigs b/tests/task_exit_killed.eigs new file mode 100644 index 0000000..a589601 --- /dev/null +++ b/tests/task_exit_killed.eigs @@ -0,0 +1,9 @@ +# #493: task_kill is a deliberate teardown, NOT an uncaught error — an unjoined +# killed worker must NOT fail the process (rc 0). +define waitmsg() as: + m is task_recv of null + return 0 +id is task_spawn of waitmsg +task_yield of null +task_kill of id +print of "MARK_END" diff --git a/tests/task_exit_unjoined_death.eigs b/tests/task_exit_unjoined_death.eigs new file mode 100644 index 0000000..45d26cd --- /dev/null +++ b/tests/task_exit_unjoined_death.eigs @@ -0,0 +1,9 @@ +# #493: a worker that dies of an uncaught error while NOBODY joins it must +# fail the process (exit non-zero) — a fire-and-forget worker's death must +# not silently green a run. Expected: stderr trace + rc 1. +define die() as: + missing +id is task_spawn of die +print of "main continues" +task_yield of null +print of "MARK_END" diff --git a/tests/test_tasks.eigs b/tests/test_tasks.eigs index aa5c1c6..3d15662 100644 --- a/tests/test_tasks.eigs +++ b/tests/test_tasks.eigs @@ -6,6 +6,27 @@ load_file of "lib/test.eigs" +# --- #510: the arena guard on suspending builtins is independent of whether a +# scheduler exists yet. This block runs BEFORE the first task_spawn, so +# g_task_sched is still NULL — the "no-op with no tasks" rule must NOT hide +# the "raise inside an arena" rule. (Keep this first; a spawn arms the +# scheduler permanently.) --- +define expect_arena_raise(thunk, label) as: + arena_mark of null + raised is 0 + try: + thunk of null + catch e: + raised is 1 + assert_eq of [e.kind, "value", label] + arena_reset of null + assert_eq of [raised, 1, label] +expect_arena_raise of [task_yield, "task_yield raises in arena with no scheduler (#510)"] +expect_arena_raise of [task_recv, "task_recv raises in arena with no scheduler (#510)"] +define sleep1(x) as: + return task_sleep of 1 +expect_arena_raise of [sleep1, "task_sleep raises in arena with no scheduler (#510)"] + # --- spawn returns a numeric id; a fresh task is alive --- define worker() as: return 42 @@ -138,6 +159,16 @@ define after_arena() as: return len of s aa is task_spawn of after_arena assert_eq of [task_join of aa, 17, "task after an arena-dier gets a clean arena"] +# Observe the arena-dier's (now DEAD) death so THIS all-pass file exits rc 0. +# An UNJOINED uncaught task death fails the process by design (#493) — that +# exit-code contract is asserted by a dedicated suite harness, not here. +threw_ad is 0 +try: + task_join of ad +catch e: + threw_ad is 1 + assert_eq of [e.kind, "value", "arena-dier died of the arena value-error"] +assert_eq of [threw_ad, 1, "an arena-dier's death is observable via task_join"] # ============ increment 2: mailboxes ============