diff --git a/CHANGELOG.md b/CHANGELOG.md index e05a297..b8a5d8b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,8 +51,19 @@ All notable changes to EigenScript are documented here. preemption between the acquire check passing and the claim. Surfaced by the `eddy` consumer (concurrency-control DST): a critical section that spans a yield needs an explicit lock, since only a non-yielding region is implicitly - atomic. Still open under #488: a debug-mode *assertion* that a delimited - region issued no scheduler yield (needs VM support). + atomic. +- **`must_not_yield of fn` — assert a critical region is atomic (#488).** The + other half of #488: a region relies on cooperative atomicity (mutual + exclusion for free) *only* while it issues no yield, but nothing marked such + a region, so a `task_yield` — or a builtin that yields internally — introduced + later broke atomicity **silently** (a rare, seed-dependent anomaly). + `must_not_yield of fn` runs `fn of null` as an atomic section: any suspending + task builtin inside (`task_yield`, a blocking `task_recv`/`task_join`, + `task_sleep`) raises `value` instead of suspending, so the mistake fails + loudly. Non-suspending ops (`task_try_recv`, `task_send`, joining a finished + task) are allowed. The region depth is balanced even when the body raises, + and it nests. Surfaced by the `eddy` consumer's snapshot-isolation commit + loop (atomic only because it contains no yield). ### Changed - **`deadlock` is now catchable (#509).** A cooperative-task deadlock (all diff --git a/docs/BUILTINS.md b/docs/BUILTINS.md index 0ecf204..1359bfe 100644 --- a/docs/BUILTINS.md +++ b/docs/BUILTINS.md @@ -571,6 +571,7 @@ Requires full build. Transformer model inference and training. | `task_spawn` | `task_spawn of fn` or `task_spawn of [fn, arg1, ...]` | Create a cooperative task (#408) running `fn` on the single OS thread — deterministic by construction, unlike `spawn`'s OS thread. Args are deep-COPIED (share-nothing, like channel sends), not shared by reference. Returns a numeric task id. (Increment 1a: the task is recorded and reported by `task_alive`; the copying-stack scheduler that runs and interleaves tasks — `task_yield`/`task_join` — lands in a later increment.) | | `task_alive` | `task_alive of id` | Returns 1 while the task is runnable or suspended, 0 once it has finished (or for an unknown id). | | `task_yield` | `task_yield of null` | Cooperatively hand control to the next ready task; this task resumes round-robin. A no-op when no task has been spawned. Forbidden inside an `arena_mark`…`arena_reset` scope or a nested evaluation (raises `value`). | +| `must_not_yield` | `must_not_yield of fn` | Run `fn of null` as an **atomic** critical section, asserting it issues no scheduler yield (#488). Any *suspending* task builtin inside — `task_yield`, a blocking `task_recv`/`task_join`, `task_sleep` — raises `value` instead of suspending, so a yield introduced into a region that relies on cooperative atomicity fails loudly rather than corrupting under a rare interleaving. Non-suspending ops (`task_try_recv`, `task_send`, joining an already-finished task) are allowed. Returns `fn`'s result (or propagates its error); the region depth is balanced even if the body raises. Nestable. | | `task_join` | `task_join of id` | Block until task `id` finishes, then return its deep-copied result — or re-raise its uncaught error (as the same `{kind, message, line}` it died with). Joining an already-finished task returns immediately; an unknown id (or self) returns null. All tasks blocked with none runnable is a `deadlock` error, not a hang — catchable by a `try`/`catch` around the join on the main task (`e.kind == "deadlock"`); terminal only if unhandled. | | `task_send` | `task_send of [id, value]` | Append a deep-copied message to task `id`'s unbounded FIFO mailbox, waking it if it waits in `task_recv`. Returns 1 if delivered, 0 if `id` is finished/unknown (a silent drop — send-to-dead is never an error). Never blocks. | | `task_recv` | `task_recv of null` | Return the next message from this task's mailbox, or block cooperatively until one arrives. Forbidden inside an `arena_mark`…`arena_reset` scope or a nested evaluation (raises `value`). | diff --git a/src/builtins.c b/src/builtins.c index 7530845..9848c34 100644 --- a/src/builtins.c +++ b/src/builtins.c @@ -4731,6 +4731,27 @@ Value* builtin_task_spawn(Value *arg) { return make_num((double)id); } +/* #488: a `must_not_yield` region asserts atomicity. A critical section under + * cooperative scheduling is atomic *only* while it issues no yield — the + * implicit mutual exclusion a non-yielding region gets for free. Nothing marked + * such a region, so a yield introduced later (directly, or via a builtin that + * yields internally) broke atomicity silently. While this depth is nonzero, + * any task builtin that would actually SUSPEND raises instead, so the mistake + * fails loudly. Only nonzero for the running task — a yield cannot happen + * inside, so no task switch occurs while it is set, and a plain counter is + * safe. */ +static int g_no_yield_depth = 0; + +static int no_yield_forbidden(const char *what) { + if (g_no_yield_depth > 0) { + rt_error(EK_VALUE, 0, + "%s inside a must_not_yield region — the critical section " + "would suspend, breaking its atomicity", what); + return 1; + } + return 0; +} + /* task_yield of null — cooperatively hand control to the next ready task. * Returns null (the value of the yield expression); the scheduler saves and * re-enqueues this task at the tail. Forbidden inside an active arena scope @@ -4749,6 +4770,7 @@ Value* builtin_task_yield(Value *arg) { return make_null(); } if (!g_task_sched) return make_null(); /* no scheduler: yield is a no-op */ + if (no_yield_forbidden("task_yield")) return make_null(); /* #488 */ task_request_yield(); return make_null(); /* placeholder: execution resumes right after this */ } @@ -4785,6 +4807,7 @@ Value* builtin_task_join(Value *arg) { "(arena_mark…arena_reset)"); return make_null(); } + if (no_yield_forbidden("blocking task_join")) return make_null(); /* #488 */ if (!task_request_join(target)) return make_null(); return make_null(); /* placeholder: the scheduler fills it with the result on resume */ } @@ -4821,6 +4844,7 @@ Value* builtin_task_recv(Value *arg) { return make_null(); } if (!g_task_sched) return make_null(); /* no tasks: nothing can arrive */ + if (no_yield_forbidden("blocking task_recv")) return make_null(); /* #488 */ task_request_recv(); return make_null(); /* placeholder: the scheduler delivers the message on resume */ } @@ -4871,10 +4895,32 @@ Value* builtin_task_sleep(Value *arg) { rt_error(EK_TYPE, 0, "task_sleep requires a number of ticks"); return make_null(); } + if (no_yield_forbidden("task_sleep")) return make_null(); /* #488 */ task_request_sleep(arg->data.num); return make_null(); /* placeholder: execution resumes when the clock wakes it */ } +/* must_not_yield of fn — run `fn of null` as an ATOMIC critical section and + * assert it issues no scheduler yield (#488). Any suspending task builtin + * inside — task_yield, a blocking task_recv/task_join, task_sleep — raises + * instead of suspending, so a yield introduced into a region that relies on + * cooperative atomicity (e.g. eddy's snapshot-isolation commit loop) fails + * loudly rather than corrupting silently under a rare interleaving. Returns + * fn's result, or propagates its error; the region depth is balanced even when + * the body raises (call_eigs_fn returns control either way). Nestable. */ +Value* builtin_must_not_yield(Value *arg) { + if (!arg || (arg->type != VAL_FN && arg->type != VAL_BUILTIN)) { + rt_error(EK_TYPE, 0, "must_not_yield requires a function"); + return make_null(); + } + g_no_yield_depth++; + Value *nul = make_null(); + Value *r = call_eigs_fn(arg, nul); + val_decref(nul); + g_no_yield_depth--; + return r ? r : make_null(); +} + /* task_now of null → the current virtual-clock value (a number, 0 before any * task_sleep and 0 when no scheduler is active). Deterministic; reads a logical * counter, so it records no tape nondet. */ @@ -5948,6 +5994,7 @@ void register_builtins(Env *env) { env_set_local_owned(env, "task_spawn", make_builtin(builtin_task_spawn)); env_set_local_owned(env, "task_alive", make_builtin(builtin_task_alive)); env_set_local_owned(env, "task_yield", make_builtin(builtin_task_yield)); + env_set_local_owned(env, "must_not_yield", make_builtin(builtin_must_not_yield)); env_set_local_owned(env, "task_join", make_builtin(builtin_task_join)); env_set_local_owned(env, "task_send", make_builtin(builtin_task_send)); env_set_local_owned(env, "task_recv", make_builtin(builtin_task_recv)); diff --git a/tests/test_tasks.eigs b/tests/test_tasks.eigs index 8873f42..16722a2 100644 --- a/tests/test_tasks.eigs +++ b/tests/test_tasks.eigs @@ -384,4 +384,50 @@ catch e: assert_eq of [e.message, "all tasks are blocked — deadlock", "deadlock carries its message"] assert_eq of [dl_caught, 1, "the deadlock handler ran and code after the try continues"] +# --- #488: must_not_yield asserts a critical region issues no scheduler yield --- +# A yield inside the region raises a catchable `value` error. +define _mny_yielder() as: + task_yield of null + return 0 +define _mny_probe_yield() as: + caught is 0 + try: + must_not_yield of _mny_yielder + catch e: + caught is 1 + assert_eq of [e.kind, "value", "must_not_yield violation is a catchable value error"] + # depth is balanced even though the body raised: a yield now must work + task_yield of null + return caught +_mny_w0 is task_spawn of _mny_probe_yield +assert_eq of [task_join of _mny_w0, 1, "yield inside must_not_yield is caught, region depth restored"] + +# A region that issues no yield returns its body's result, no raise. +_mny_acc is 0 +define _mny_clean() as: + _mny_acc is _mny_acc + 5 + _mny_acc is _mny_acc + 5 + return _mny_acc +assert_eq of [must_not_yield of _mny_clean, 10, "non-yielding region returns its body result"] + +# Non-blocking ops (task_try_recv) inside a region do NOT trip the assertion. +define _mny_nonblock() as: + m is task_try_recv of null + return 1 +assert_eq of [must_not_yield of _mny_nonblock, 1, "non-suspending ops are allowed in a region"] + +# A blocking task_recv inside a region raises (would suspend). +define _mny_blockrecv() as: + m is task_recv of null + return 0 +define _mny_probe_recv() as: + caught is 0 + try: + must_not_yield of _mny_blockrecv + catch e: + caught is 1 + return caught +_mny_w1 is task_spawn of _mny_probe_recv +assert_eq of [task_join of _mny_w1, 1, "blocking task_recv inside must_not_yield raises"] + test_summary of null