feat(tasks): must_not_yield — assert a critical region is atomic (#488)#528
Merged
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| obj->data.ident.name, (op[0] == '=' ? "==" : op), lit, sugg); | ||
| } | ||
|
|
||
| static void w018_scan(ASTNode *n, LintContext *ctx, W018Scope *sc) { |
The other half of #488 (the lib/sync locks landed in #489). A region under cooperative scheduling is atomic — 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: the program still ran and usually looked fine, failing only as a rare, seed-dependent anomaly. `must_not_yield of fn` runs `fn of null` as an atomic critical section and asserts it. A new g_no_yield_depth is incremented for the region; while nonzero, every task builtin that would actually SUSPEND raises `value` instead: - task_yield (when a scheduler is active), - blocking task_recv / blocking task_join (a live target), - task_sleep. Non-suspending ops stay legal in a region (task_try_recv, task_send, joining an already-finished task) — the guard sits on the actual-suspend path, so it never false-fires. The depth is only ever nonzero for the running task (a yield can't happen inside), so a plain counter is safe, and it is balanced even when the body raises (call_eigs_fn returns control either way). Nestable. Surfaced by the eddy consumer: its snapshot-isolation commit loop is correct only because it contains no yield. Regression (tests/test_tasks.eigs): a yield inside is caught (`value`) and the region depth is restored (a later yield works); a non-yielding region returns its body result; non-suspending ops are allowed; a blocking task_recv inside raises. Documented in docs/BUILTINS.md; discoverability gate satisfied. Full suite green release + ASan (leaks on) + jit-smoke, 2703/2703, no leak-tally increase. Closes #488 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
InauguralPhysicist
force-pushed
the
feat/must-not-yield-488
branch
from
80f075d to
5908412
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The other half of #488 (the
lib/synclocks landed in #489). A region under cooperative scheduling is atomic — mutual exclusion for free — only while it issues no yield, but nothing marked such a region. So atask_yield(or a builtin that yields internally) introduced later broke atomicity silently: the program still ran and usually looked fine, failing only as a rare, seed-dependent anomaly.must_not_yield of fnrunsfn of nullas an atomic critical section and asserts it.How
A new
g_no_yield_depthis incremented for the region; while nonzero, every task builtin that would actually suspend raisesvalueinstead:task_yield(when a scheduler is active),task_recv/ blockingtask_join(a live target),task_sleep.Non-suspending ops stay legal (
task_try_recv,task_send, joining an already-finished task) — the guard sits on the actual-suspend path, so it never false-fires. The depth is only ever nonzero for the running task (a yield can't happen inside), so a plain counter is safe, and it's balanced even when the body raises (call_eigs_fnreturns control either way). Nestable.Surfaced by the
eddyconsumer: its snapshot-isolation commit loop is correct only because it contains no yield.Verification
tests/test_tasks.eigs: a yield inside is caught (value) and the depth is restored (a later yield works); a non-yielding region returns its body result; non-suspending ops are allowed; a blockingtask_recvinside raises.detect_leaks=1) + jit-smoke: 2703/2703, 0 failed, no leak-tally increase.Docs
docs/BUILTINS.md:must_not_yieldrow (discoverability gate satisfied).CHANGELOG.md: entry under[Unreleased] → Added, and the earlierlib/syncnote's "still open under lib/sync: a way to mark a cooperative-task region as must-not-yield (atomic/critical) #488" clause removed.Closes #488
🤖 Generated with Claude Code