Detect unclosed cancel scopes when a task exits

newsfragments/3329.bugfix.rst

@@ -0,0 +1,3 @@
+Trio now reports an error when a task exits without closing a cancel scope it
+entered, instead of allowing that scope to be exited by another task. Deadlines
+associated with these unclosed scopes are also cleaned up.

src/trio/_core/_run.py

@@ -2026,11 +2026,21 @@ def task_exited(self, task: Task, outcome: Outcome[object]) -> None:
                 lot.break_lot(task)
             del GLOBAL_PARKING_LOT_BREAKER[task]
 
+        unclosed_scope = (
+            task._parent_nursery is not None
+            and task._cancel_status is not task._parent_nursery._cancel_status
+            and task._cancel_status is not None
+            and not task._cancel_status.abandoned_by_misnesting
+        )
         if (
-            task._cancel_status is not None
-            and task._cancel_status.abandoned_by_misnesting
-            and task._cancel_status.parent is None
-        ) or task._child_nurseries:
+            (
+                task._cancel_status is not None
+                and task._cancel_status.abandoned_by_misnesting
+                and task._cancel_status.parent is None
+            )
+            or task._child_nurseries
+            or unclosed_scope
+        ):
             reason = "Nursery" if task._child_nurseries else "Cancel scope"
             # The cancel scope surrounding this task's nursery was closed
             # before the task exited. Force the task to exit with an error,
@@ -2039,6 +2049,11 @@ def task_exited(self, task: Task, outcome: Outcome[object]) -> None:
             try:
                 # Raise this, rather than just constructing it, to get a
                 # traceback frame included
+                if unclosed_scope and not task._child_nurseries:
+                    raise RuntimeError(
+                        f"Cancel scope stack corrupted: {task!r} exited without "
+                        f"closing its cancel scope\n{MISNESTING_ADVICE}",
+                    )
                 raise RuntimeError(
                     f"{reason} stack corrupted: {reason} surrounding "
                     f"{task!r} was closed before the task exited\n{MISNESTING_ADVICE}",
@@ -2048,7 +2063,18 @@ def task_exited(self, task: Task, outcome: Outcome[object]) -> None:
                     new_exc.__context__ = outcome.error
                 outcome = Error(new_exc)
 
+        exited_status: CancelStatus | None = task._cancel_status
         task._activate_cancel_status(None)
+        if unclosed_scope and not task._child_nurseries:
+            assert task._parent_nursery is not None
+            while exited_status is not task._parent_nursery._cancel_status:
+                assert exited_status is not None
+                parent_status = exited_status.parent
+                scope = exited_status._scope
+                exited_status.close()
+                with scope._might_change_registered_deadline():
+                    scope._cancel_status = None
+                exited_status = parent_status
         self.tasks.remove(task)
         if task is self.init_task:
             # If the init task crashed, then something is very wrong and we

src/trio/_core/_tests/test_guest_mode.py

@@ -39,6 +39,21 @@
 InHost: TypeAlias = Callable[[Callable[[], object]], None]
 
 
+def test_guest_unclosed_cancel_scope_deadline_cleanup() -> None:
+    async def main(in_host: InHost) -> None:
+        scope = trio.CancelScope(deadline=trio.current_time() + 100)
+
+        async def child() -> None:
+            scope.__enter__()
+
+        with pytest.RaisesGroup(RuntimeError):
+            async with trio.open_nursery() as nursery:
+                nursery.start_soon(child)
+        assert scope._registered_deadline == inf
+
+    trivial_guest_run(main)
+
+
 # The simplest possible "host" loop.
 # Nice features:
 # - we can run code "outside" of trio using the schedule function passed to

src/trio/_core/_tests/test_run.py

@@ -852,6 +852,66 @@ async def task3(task_status: _core.TaskStatus[_core.CancelScope]) -> None:
         scope.cancel()
 
 
+@pytest.mark.parametrize("depth", [1, 2])
+@pytest.mark.parametrize("cancelled", [False, True])
+@pytest.mark.parametrize("adopted", [False, True])
+async def test_cancel_scope_left_open_at_task_exit(
+    depth: int, cancelled: bool, adopted: bool
+) -> None:
+    # A scope must not outlive the task that entered it (issue #3329).
+    scopes = [
+        _core.CancelScope(deadline=_core.current_time() + 100) for _ in range(depth)
+    ]
+
+    async def enter_scope(
+        *, task_status: _core.TaskStatus[None] = _core.TASK_STATUS_IGNORED
+    ) -> None:
+        for scope in scopes:
+            scope.__enter__()
+        task_status.started()
+        if cancelled:
+            scopes[-1].cancel()
+            await _core.checkpoint()
+
+    with pytest.RaisesGroup(RuntimeError):
+        async with _core.open_nursery() as nursery:
+            if adopted:
+                await nursery.start(enter_scope)
+            else:
+                nursery.start_soon(enter_scope)
+
+    assert all(scope._registered_deadline == inf for scope in scopes)
+    assert all(scope._cancel_status is None for scope in scopes)
+
+
+async def test_unclosed_cancel_scope_preserves_error_and_parent() -> None:
+    original = ValueError("child failure")
+    child_scope = _core.CancelScope(deadline=_core.current_time() + 100)
+
+    async def child() -> None:
+        child_scope.__enter__()
+        raise original
+
+    with _core.CancelScope(deadline=_core.current_time() + 200) as parent_scope:
+        parent_status = parent_scope._cancel_status
+        parent_deadline = parent_scope._registered_deadline
+        with pytest.RaisesGroup(
+            pytest.RaisesExc(
+                RuntimeError,
+                match="exited without closing its cancel scope",
+                check=lambda exc: exc.__context__ is original,
+            )
+        ):
+            async with _core.open_nursery() as nursery:
+                nursery.start_soon(child)
+        assert parent_scope._cancel_status is parent_status
+        assert parent_scope._registered_deadline == parent_deadline
+        assert not parent_scope.cancel_called
+        await _core.checkpoint()
+    assert child_scope._cancel_status is None
+    assert child_scope._registered_deadline == inf
+
+
 # helper to check we're not outputting overly verbose tracebacks
 def no_cause_or_context(e: BaseException) -> bool:
     return e.__cause__ is None and e.__context__ is None