Improved asyncio cancellation semantics

agronholm · agronholm · commit 04f9ce61b282 · 2022-11-06T23:08:55.000+02:00
The semantics now better match with trio's.
diff --git a/docs/versionhistory.rst b/docs/versionhistory.rst
@@ -42,6 +42,8 @@ This library adheres to `Semantic Versioning 2.0 <http://semver.org/>`_.
   the event loop to be closed
 - Fixed ``current_effective_deadline()`` not returning ``-inf`` on asyncio when the
   currently active cancel scope has been cancelled (PR by Ganden Schaffner)
+- Fixed task group not raising a cancellation exception on asyncio at exit if no child
+  tasks were spawned and an outer cancellation scope had been cancelled before
 
 **3.6.1**
 
diff --git a/src/anyio/_backends/_asyncio.py b/src/anyio/_backends/_asyncio.py
@@ -42,7 +42,6 @@
     Coroutine,
     Deque,
     Generator,
-    Iterator,
     Mapping,
     Optional,
     Sequence,
@@ -456,14 +455,6 @@ def collapse_exception_group(excgroup: BaseExceptionGroup) -> BaseException:
         return excgroup
 
 
-def walk_exception_group(excgroup: BaseExceptionGroup) -> Iterator[BaseException]:
-    for exc in excgroup.exceptions:
-        if isinstance(exc, BaseExceptionGroup):
-            yield from walk_exception_group(exc)
-        else:
-            yield exc
-
-
 def is_anyio_cancelled_exc(exc: BaseException) -> bool:
     return isinstance(exc, CancelledError) and not exc.args
 
@@ -490,11 +481,21 @@ async def __aexit__(
             self.cancel_scope.cancel()
             self._exceptions.append(exc_val)
 
-        while self.cancel_scope._tasks:
+        if self.cancel_scope._tasks:
+            while self.cancel_scope._tasks:
+                try:
+                    await asyncio.wait(self.cancel_scope._tasks)
+                except asyncio.CancelledError as e:
+                    if exc_val is None:
+                        self.cancel_scope.cancel()
+                        self._exceptions.append(e)
+                        exc_val = e
+        else:
+            # No tasks to wait on, but we still need to check for cancellation here
             try:
-                await asyncio.wait(self.cancel_scope._tasks)
-            except asyncio.CancelledError:
-                self.cancel_scope.cancel()
+                await AsyncIOBackend.checkpoint()
+            except CancelledError as e:
+                self._exceptions.append(e)
 
         self._active = False
         if self._exceptions:
@@ -504,9 +505,6 @@ async def __aexit__(
                 # If any exceptions other than AnyIO cancellation exceptions have been
                 # received, raise those
                 _, exc = group.split(is_anyio_cancelled_exc)
-            elif all(is_anyio_cancelled_exc(e) for e in walk_exception_group(group)):
-                # All tasks were cancelled by AnyIO
-                exc = CancelledError()
             else:
                 exc = group
 
diff --git a/src/anyio/from_thread.py b/src/anyio/from_thread.py
@@ -148,6 +148,10 @@ async def __aexit__(
         await self.stop()
         return await self._task_group.__aexit__(exc_type, exc_val, exc_tb)
 
+    @property
+    def _running(self) -> bool:
+        return self._event_loop_thread_id is not None
+
     def _check_running(self) -> None:
         if self._event_loop_thread_id is None:
             raise RuntimeError("This portal is not running")
@@ -202,8 +206,11 @@ def callback(f: Future) -> None:
             if not future.cancelled():
                 future.set_exception(exc)
 
-            # Let base exceptions fall through
+            # Let base exceptions fall through, but mark the portal as not running, so
+            # start_blocking_portal() won't try to stop it since BaseException will
+            # cause that anyway
             if not isinstance(exc, Exception):
+                self._event_loop_thread_id = None
                 raise
         else:
             if not future.cancelled():
@@ -413,9 +420,7 @@ async def run_portal() -> None:
                 cancel_remaining_tasks = True
                 raise
             finally:
-                try:
+                if not run_future.done() and portal._running:
                     portal.call(portal.stop, cancel_remaining_tasks)
-                except RuntimeError:
-                    pass
 
         run_future.result()
diff --git a/tests/test_taskgroups.py b/tests/test_taskgroups.py
@@ -396,6 +396,45 @@ async def test_cancel_before_entering_scope() -> None:
         pytest.fail("execution should not reach this point")
 
 
+async def test_cancel_outer_scope_no_tasks() -> None:
+    """
+    Test that a task group raises an exception group containing one cancellation error
+    from __aexit__() if the outer cancel scope was cancelled.
+
+    """
+    with CancelScope() as outer_scope:
+        try:
+            async with anyio.create_task_group():
+                outer_scope.cancel()
+        except BaseException as exc:
+            if not isinstance(exc, get_cancelled_exc_class()):
+                pytest.fail("should have raised a cancellation exception")
+
+            raise
+        else:
+            pytest.fail("should have raised an exception")
+
+
+async def test_cancel_outer_scope_one_task() -> None:
+    """
+    Test that a task group propagates a cancellation error (wrapped in an exception
+    group) from __aexit__() that was not intended for the task group's cancel scope.
+
+    """
+    with CancelScope() as outer_scope:
+        try:
+            async with anyio.create_task_group() as tg:
+                tg.start_soon(sleep, 3)
+                outer_scope.cancel()
+        except BaseExceptionGroup as excgrp:
+            assert len(excgrp.exceptions) == 2
+            raise
+        except get_cancelled_exc_class():
+            pytest.fail("task group raised a plain cancellation error")
+        else:
+            pytest.fail("should have raised an exception group")
+
+
 async def test_exception_group_children() -> None:
     with pytest.raises(BaseExceptionGroup) as exc:
         async with create_task_group() as tg:
