Skip to content

fix: do not swallow concurrent native Task.cancel on CancelScope exit (#1214) - #1251

Open
MohammedAnasNathani wants to merge 2 commits into
agronholm:masterfrom
MohammedAnasNathani:fix/cancelscope-external-cancel
Open

fix: do not swallow concurrent native Task.cancel on CancelScope exit (#1214)#1251
MohammedAnasNathani wants to merge 2 commits into
agronholm:masterfrom
MohammedAnasNathani:fix/cancelscope-external-cancel

Conversation

@MohammedAnasNathani

Copy link
Copy Markdown

Changes

On the asyncio backend, when a CancelScope cancels its host task and a concurrent native Task.cancel() lands in the same event-loop cycle, __exit__ could swallow the AnyIO-tagged CancelledError after undoing only the scope's own cancel counts. The host then returned normally with task.cancelled() is False and task.cancelling() > 0, so callers awaiting the task never saw cancellation.

This is the Happy Eyeballs / connect_tcp pattern from production: a child cancels the task group while an unrelated caller cancels the host task.

Fix

  1. Snapshot Task.cancelling() when the scope is entered.
  2. On exit, after calling uncancel() for each cancel this scope delivered to the host, do not swallow if cancelling() is still above that baseline — a native cancel that arrived while the scope was active must still propagate.
  3. Cancels that were already pending before enter are ignored for this check (preserves test_cancel_message_replaced and related uncancel semantics).

Checklist

Related

Fixes #1214

Verification
# Issue repro (before: returned normally / cancelled=False / cancelling=1)
# After: CancelledError propagates / cancelled=True / cancelling=1

pytest tests/test_taskgroups.py::TestUncancel   # 30 passed
pytest tests/test_taskgroups.py                 # 539 passed, 153 skipped, 4 xfailed

When a cancel scope delivers cancellation to its host task (e.g. Happy
Eyeballs winning and cancelling the task group) and a native
Task.cancel() lands in the same cycle, __exit__ previously uncancelled
only the AnyIO-tagged cancels and then swallowed the AnyIO
CancelledError. That left cancelling() > 0 with no exception
propagating — the host returned normally and callers waiting on the
task hung.

Snapshot Task.cancelling() at scope enter and, after undoing this
scope's pending uncancels, refuse to swallow if the cancel count is
still above that baseline. Pre-existing cancels from before enter are
unaffected.

Fixes agronholm#1214
Copilot AI review requested due to automatic review settings July 29, 2026 14:43

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@agronholm

Copy link
Copy Markdown
Owner

The test already passes on master, so what's going on here?

Comment thread tests/test_taskgroups.py
Comment on lines +1692 to +1734
async def test_child_scope_cancel_does_not_swallow_native_host_cancel(
self,
) -> None:
"""
When a child cancels its task group (Happy Eyeballs style) and a native
``Task.cancel()`` lands on the host in the same cycle, the host must still
observe ``CancelledError`` — not return normally with ``cancelling() > 0``.

Regression test for #1214.
"""
attempt_started = asyncio.Event()
connection_won = asyncio.Event()

async def operation() -> None:
async with create_task_group() as task_group:

async def connect_attempt() -> None:
attempt_started.set()
await connection_won.wait()
# Same pattern as connect_tcp(): winner cancels the group.
task_group.cancel_scope.cancel()

task_group.start_soon(connect_attempt)
await sleep_forever()

task = asyncio.create_task(operation())
await attempt_started.wait()

external_cancel_accepted: list[bool] = []

def externally_cancel() -> None:
external_cancel_accepted.append(task.cancel("external cancellation"))

connection_won.set()
asyncio.get_running_loop().call_soon(externally_cancel)

with pytest.raises(asyncio.CancelledError):
await task

assert external_cancel_accepted == [True]
assert task.cancelled()
assert task.cancelling() >= 1

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is a massively overcomplicated. The bug can be reproduced with 1/5 of the code here.

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.

CancelScope can swallow a concurrent native asyncio.Task.cancel() when the scope is also cancelled

3 participants