🐛 propagate halt out of scoped() + race() when callcc resolves externally#1187
Closed
taras wants to merge 1 commit into
Closed
🐛 propagate halt out of scoped() + race() when callcc resolves externally#1187taras wants to merge 1 commit into
taras wants to merge 1 commit into
Conversation
…ally Fixes #1185. scoped() runs on the caller's coroutine, and its async teardown (destroy() halting race children) spans multiple scheduler turns. The single-use unwinding flag was already consumed by the time teardown finished, so the outer loop resumed via iterator.next() and ran to completion instead of halting — breaking SIGINT shutdown for the main()/scoped()/race() pattern. Route the scoped body through a Trap, run teardown as critical(destroy), then re-assert the unwind via the trap's exit() once teardown completes. This keeps the fix inside scoped()/trap() and preserves the single-use-return unwind contract (no change to the coroutine step() state machine). Adds a regression test reproducing the callcc + scoped + race halt path.
commit: |
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
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.
Fixes #1185.
Problem
When
callccis resolved externally (scope.run(() => resolve(...))) inside ascoped()body that usesrace(), the halt signal failed to stop the surrounding loop — all rounds ran to completion. This broke graceful SIGINT shutdown for the exactmain()+scoped()round-boundary +race()-timeout pattern.Root cause
scoped()runs on the same coroutine as its caller. On halt,routine.unwind()setsdata.unwinding = true, andstep()consumes it exactly once — it callsiterator.return()and resets the flag. Butscoped's teardown (yield* destroy()) is async: it halts theracechildren across multiple scheduler turns. By the time teardown finished,unwindinghad already been reset, sostep()took the normaliterator.next()path and the outer loop simply continued to the next round instead of finishing its own unwind. (race/callccmatter only because they introduce the spawned children that make teardown span multiple turns.)Fix
Localize the fix to
scoped()/trap()and reuse the existing unwind-propagation machinery — no change to the coroutinestep()state machine:Trap(capturing success/error int.outcome).yield* critical(destroy)so multi-step async cleanup completes via.next()rather than being short-circuited by the unwind branch.yield t.exit()— when no outcome exists (the body was interrupted),Trap.exit()callsroutine.unwind()again, re-asserting the unwind so the outer loop finishes instead of continuing.This preserves the single-use-
returnunwind contract (thetest/coroutine.test.ts"uses 'return' for a single iteration when unwound" canary still passes) andexportsTrapsoscopedcan reuse it.Relationship to #1186
This implements the same approach as #1186 (Trap-routed
scoped+critical(destroy)+t.exit()) and additionally adds the missing regression test pinning thecallcc+scoped+racehalt path — there was previously no test covering this exact combination, so the bug could silently return.Test
test/scoped.test.tsadds "propagates halt out of a scoped() + race() loop when callcc resolves externally". It reproduces the issue's scenario: the loop must stop afterround-1-teardownand never start round 2. Verified to fail onv4HEAD (all 3 rounds run) and pass with this fix.