Visitar URL original
Don't drop SIGINT that a task can't take right away by mtsolmn · Pull Request #104 · google/duet · GitHub
Skip to content

Don't drop SIGINT that a task can't take right away - #104

Open
mtsolmn wants to merge 2 commits into
google:mainfrom
mtsolmn:fix-sigint-interruptible-task
Open

mtsolmn wants to merge 2 commits into
google:mainfrom
mtsolmn:fix-sigint-interruptible-task

Conversation

@mtsolmn

@mtsolmn mtsolmn commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

Problem

When SIGINT arrives while the scheduler is between ticks, Scheduler._interrupt sets _interrupted. The next tick() then delivers a KeyboardInterrupt to next(iter(self.active_tasks)), an arbitrary task, and clears the flag. But Task.interrupt silently does nothing if the task is not interruptible or already has an interrupt pending. While a scope cleans up after an error, its main task is uninterruptible (it waits in finish_tasks()) and its other tasks already have interrupts pending, so a Ctrl-C in that window was dropped and the program kept running. A Ctrl-C could also be lost after reaching a task whose main task couldn't take it, or if the run ended before any task took it. We hit this with Ctrl-C hangs in a downstream project.

Reproduction

import signal
import duet

async def main():
    task = duet.impl.current_task()
    task.interruptible = False
    task.scheduler._interrupt(signal.SIGINT, None)  # Simulate Ctrl-C between ticks.
    await duet.completed_future(None)  # Tick: no task can take the interrupt.
    task.interruptible = True
    await duet.completed_future(None)  # main: the SIGINT is already gone.

duet.run(main)  # main: returns normally. Expected: KeyboardInterrupt.

Fix

  • Task.interrupt now returns whether it accepted the interrupt (existing callers ignore the result).
  • tick() offers the KeyboardInterrupt to the active tasks in turn and stops at the first that accepts it. If none does, _interrupted stays set and a later tick tries again. This can't busy-loop, because get_all() still blocks until some task is ready. The flag is cleared only when a task takes the KeyboardInterrupt.
  • A task that fails with KeyboardInterrupt reports it to its main task via Task.interrupt, which used to drop it if the main task couldn't take an interrupt right then (e.g. while finishing its tasks after another error; the scope then re-raised the original error and an except Exception: upstream carried on). The task now re-posts the SIGINT (Scheduler.repost_interrupt), so the next tick delivers it to a task that can take it.
  • If a SIGINT is still pending when the run finishes, duet.run now raises KeyboardInterrupt instead of returning normally. It takes the SIGINT with the new Scheduler.take_interrupt(), which also clears the flag, so it isn't delivered twice. If the run fails with another error while a SIGINT is pending, that error is raised as before; the SIGINT isn't raised in addition.

Tests

  • New: TestScheduler.test_sigint_interrupts_interruptible_task (in impl_test.py). It makes iteration over active_tasks put the uninterruptible main task first, then checks that the interruptible child gets the KeyboardInterrupt.
  • New: TestScheduler.test_sigint_stays_pending_while_no_task_is_interruptible.
  • New: TestScheduler.test_keyboard_interrupt_kept_pending_if_main_task_cannot_take_it and TestScheduler.test_sigint_pending_when_run_finishes_is_raised.
  • All four fail on main (DID NOT RAISE KeyboardInterrupt) and pass with this change. SIGINT is simulated by calling scheduler._interrupt(signal.SIGINT, None) between ticks, so no real signals, threads or timing are involved.

@mtsolmn mtsolmn changed the title Deliver SIGINT to an interruptible task Don't drop SIGINT that a task can't take right away Oct 31, 2026
@mtsolmn
mtsolmn force-pushed the fix-sigint-interruptible-task branch from a326c7c to 65b74e8 Compare October 31, 2026 02:20
mtsolmn added a commit to mtsolmn/duet that referenced this pull request Oct 31, 2026
If an async generator holds a scope open across a yield, e.g.

    async def generate():
        async with duet.new_scope() as scope:
            async for x in duet.pmap_aiter(scope, func, items):
                yield x

and the caller stops iterating early (break, or an exception) without
calling aclose(), the generator is closed synchronously when it is garbage
collected, throwing GeneratorExit into new_scope. new_scope then
interrupted the remaining tasks and awaited them, but a generator cannot
yield while handling GeneratorExit, so this failed with an unraisable
"RuntimeError: async generator ignored GeneratorExit". Worse, the abandoned
new_scope frame had already set main_task.interruptible = False and never
restored it, so the caller could no longer be interrupted, e.g. by a
timeout: a following duet.sleep() hung forever. And because the
interrupted tasks were still attached to the caller's task, their eventual
failure would interrupt the caller, which had moved on, with
RuntimeError("scope exited").

On GeneratorExit, interrupt the remaining tasks without awaiting them, and
detach them from main_task with a new Task.detach() method, so that their
failure is dropped instead of reported to main_task. Detached tasks are
interrupted from their new main task, DETACHED, so that (with the fix that
keeps parent-scope Interrupts intact in nested scopes) the Interrupt cannot
be turned into a catchable RuntimeError inside them.

Behaviour change: when the generator is closed with `await agen.aclose()`,
the tasks are now interrupted but finish in the background rather than
before aclose() returns.

This is robust only together with the fixes that preserve parent-scope
Interrupts in nested scopes and that re-register ready tasks when a root
task raises in Scheduler.tick.

DETACHED refuses interrupts, like a task that can't be interrupted, so
that once the scheduler re-posts a KeyboardInterrupt its main task can't
take (google#104), a Ctrl-C that reaches a detached task is not lost.
@mtsolmn
mtsolmn marked this pull request as ready for review October 31, 2026 02:37
Comment thread duet/impl.py Resolved
Comment thread duet/impl.py Resolved
Comment thread duet/api.py Resolved
When SIGINT arrives while the scheduler is waiting between ticks, the next
tick delivers a KeyboardInterrupt to a task. It picked an arbitrary task,
next(iter(self.active_tasks)), and cleared the pending flag. But
Task.interrupt silently ignores tasks that are not interruptible or already
have an interrupt pending. While a scope cleans up after an error, its main
task is uninterruptible and its other tasks already have interrupts
pending, so a Ctrl-C in that window was dropped and the program kept
running.

Make Task.interrupt return whether it accepted the interrupt, and in tick
offer the KeyboardInterrupt to the active tasks until one accepts it. If
none does, keep the SIGINT pending and try again on the next tick.

A Ctrl-C could still be lost in two more ways, now also fixed:
- A task that fails with KeyboardInterrupt reports it to its main task,
  which drops it if it can't take an interrupt right now (e.g. it is
  finishing its tasks after another error). Re-post the SIGINT instead, so
  the next tick delivers it to a task that can take it.
- If the run finishes while a SIGINT is still pending, duet.run now raises
  KeyboardInterrupt instead of returning normally.
@mtsolmn
mtsolmn force-pushed the fix-sigint-interruptible-task branch from 65b74e8 to bceb8f7 Compare November 6, 2026 19:47
mtsolmn added a commit to mtsolmn/duet that referenced this pull request Nov 6, 2026
If an async generator holds a scope open across a yield, e.g.

    async def generate():
        async with duet.new_scope() as scope:
            async for x in duet.pmap_aiter(scope, func, items):
                yield x

and the caller stops iterating early (break, or an exception) without
calling aclose(), the generator is closed synchronously when it is garbage
collected, throwing GeneratorExit into new_scope. new_scope then
interrupted the remaining tasks and awaited them, but a generator cannot
yield while handling GeneratorExit, so this failed with an unraisable
"RuntimeError: async generator ignored GeneratorExit". Worse, the abandoned
new_scope frame had already set main_task.interruptible = False and never
restored it, so the caller could no longer be interrupted, e.g. by a
timeout: a following duet.sleep() hung forever. And because the
interrupted tasks were still attached to the caller's task, their eventual
failure (ScopeExit) would interrupt the caller, which had moved on.

On GeneratorExit, interrupt the remaining tasks with ScopeExit without
awaiting them, and detach them from main_task with a new Task.detach()
method, so that their failure is dropped instead of reported to main_task.

Behaviour change: when the generator is closed with `await agen.aclose()`,
the tasks are now interrupted but finish in the background rather than
before aclose() returns.

Detached tasks are exactly the kind of tasks that no scope waits for, so
this is robust only together with the fix that re-registers ready tasks
when a root task raises in Scheduler.tick (google#103).

DETACHED refuses interrupts, like a task that can't be interrupted, so
that once the scheduler re-posts a KeyboardInterrupt its main task can't
take (google#104), a Ctrl-C that reaches a detached task is not lost.
- Only clear the pending SIGINT flag in tick when a task takes the
  KeyboardInterrupt.
- Rename accepted to interrupted.
- Clear the pending SIGINT flag when duet.run raises KeyboardInterrupt for
  it, so it is not delivered twice (Scheduler.take_interrupt replaces the
  interrupt_pending property).
mtsolmn added a commit to mtsolmn/duet that referenced this pull request Nov 10, 2026
If an async generator holds a scope open across a yield, e.g.

    async def generate():
        async with duet.new_scope() as scope:
            async for x in duet.pmap_aiter(scope, func, items):
                yield x

and the caller stops iterating early (break, or an exception) without
calling aclose(), the generator is closed synchronously when it is garbage
collected, throwing GeneratorExit into new_scope. new_scope then
interrupted the remaining tasks and awaited them, but a generator cannot
yield while handling GeneratorExit, so this failed with an unraisable
"RuntimeError: async generator ignored GeneratorExit". Worse, the abandoned
new_scope frame had already set main_task.interruptible = False and never
restored it, so the caller could no longer be interrupted, e.g. by a
timeout: a following duet.sleep() hung forever. And because the
interrupted tasks were still attached to the caller's task, their eventual
failure (ScopeExit) would interrupt the caller, which had moved on.

On GeneratorExit, interrupt the remaining tasks with ScopeExit without
awaiting them, and detach them from main_task with a new Task.detach()
method, so that their failure is dropped instead of reported to main_task.

Behaviour change: when the generator is closed with `await agen.aclose()`,
the tasks are now interrupted but finish in the background rather than
before aclose() returns.

Detached tasks are exactly the kind of tasks that no scope waits for, so
this is robust only together with the fix that re-registers ready tasks
when a root task raises in Scheduler.tick (google#103).

_DETACHED refuses interrupts, like a task that can't be interrupted, so
that once the scheduler re-posts a KeyboardInterrupt its main task can't
take (google#104), a Ctrl-C that reaches a detached task is not lost.
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.

2 participants