Skip to content

fix(cancel): instant Notify-backed AbortSignal + pin the drop contract - #210

Merged
yogthos merged 1 commit into
mainfrom
fix/instant-cancellation
May 29, 2026
Merged

fix(cancel): instant Notify-backed AbortSignal + pin the drop contract#210
yogthos merged 1 commit into
mainfrom
fix/instant-cancellation

Conversation

@yogthos

@yogthos yogthos commented May 29, 2026

Copy link
Copy Markdown
Collaborator

Summary

Thread 3, item #9dirge-2mw0.

Investigating "a cancelled tool keeps running detached" showed the dispatcher's drop-on-cancel is actually correct: tokio::select! dropping the tool's execute future cancels it — no further polls, in-flight .awaits unwind, and RAII guards run (e.g. bash's PgKillGuard already SIGKILLs the whole process group on the drop path). MCP has no detached spawn either. The real shortcomings were responsiveness and an over-pessimistic comment — plus no test pinning the contract.

Changes

  • AbortSignal is now Notify-backed. cancel() notifies waiters; cancelled().await resolves the instant cancellation fires — race-free via Notified::enable() before the state check. The cheap is_cancelled() poll API is unchanged for tools.
  • wait_for_cancel awaits cancelled() instead of a 50ms poll loop → Ctrl+C is immediate, no busy-poll.
  • Corrected the dispatcher comment to state accurately that dropping the future cancels it (and bash kills its group on drop); the genuine residual is tools that detach via tokio::spawn, which must abort that work themselves.

Test plan

  • AbortSignal::cancelled() resolves-when-already-cancelled + wakes-on-concurrent-cancel.
  • A dispatcher regression proving the tool future is dropped on a mid-execution cancel (RAII drop-flag set, completion flag NOT set) — i.e. no detached execution.

2115 pass under the full feature matrix at RUSTFLAGS="-D warnings".

…t (dirge-2mw0)

Investigating "a cancelled tool keeps running detached" showed the
dispatcher's drop-on-cancel is actually correct: `tokio::select!`
dropping the tool's execute future cancels it (no further polls, awaits
unwind, RAII guards run — e.g. bash's PgKillGuard SIGKILLs the whole
process group). The real shortcomings were responsiveness + an
over-pessimistic comment, plus no test pinning the contract.

Changes:
- AbortSignal is now Notify-backed. `cancel()` notifies waiters;
  `cancelled().await` resolves the instant cancellation fires
  (race-free via `Notified::enable()` before the state check). The
  cheap `is_cancelled()` poll API is unchanged for tools.
- wait_for_cancel awaits `cancelled()` instead of a 50ms poll loop —
  Ctrl+C is now immediate, with no busy-poll.
- Corrected the dispatcher comment to state accurately that dropping
  the future cancels it (and bash kills its group on the drop path);
  the genuine residual is tools that detach via tokio::spawn, which
  must abort that work themselves.

Tests: AbortSignal.cancelled() resolves-when-already-cancelled +
wakes-on-concurrent-cancel; and a dispatcher regression proving the
tool future is DROPPED on a mid-execution cancel (RAII drop-flag set,
completion flag NOT set) — i.e. no detached execution. 2115 pass under
the full feature matrix at -D warnings.
@yogthos
yogthos merged commit cd3f509 into main May 29, 2026
9 checks passed
@yogthos
yogthos deleted the fix/instant-cancellation branch May 29, 2026 17:23
allen-munsch pushed a commit to allen-munsch/dirge that referenced this pull request Jun 3, 2026
…ation

fix(cancel): instant Notify-backed AbortSignal + pin the drop contract
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant