From e334c94ae316bd02fa9f5b8cc5de4e9f59bf40c8 Mon Sep 17 00:00:00 2001 From: A5rocks Date: Wed, 3 Sep 2025 23:01:19 +0900 Subject: [PATCH 1/8] Suppress `.aclose()`'s GeneratorExit in exception groups --- src/trio/_channel.py | 13 +++++++++++-- src/trio/_tests/test_channel.py | 29 ++++++++++++++++++++++++++++- 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/src/trio/_channel.py b/src/trio/_channel.py index 9caa7f7699..6f81d563b2 100644 --- a/src/trio/_channel.py +++ b/src/trio/_channel.py @@ -570,6 +570,8 @@ async def _move_elems_to_channel( # `async with send_chan` will eat exceptions, # see https://github.com/python-trio/trio/issues/1559 with send_chan: + # replace try-finally with contextlib.aclosing once python39 is + # dropped: try: task_status.started() while True: @@ -582,7 +584,14 @@ async def _move_elems_to_channel( # Send the value to the channel await send_chan.send(value) finally: - # replace try-finally with contextlib.aclosing once python39 is dropped - await agen.aclose() + # work around `.aclose()` not suppressing GeneratorExit in an + # ExceptionGroup: + # TODO: make an issue on CPython about this + try: + await agen.aclose() + except BaseExceptionGroup as eg: + _, eg = eg.split(GeneratorExit) + if eg is not None: + raise eg return context_manager diff --git a/src/trio/_tests/test_channel.py b/src/trio/_tests/test_channel.py index f1556a153c..7c4e0b00a5 100644 --- a/src/trio/_tests/test_channel.py +++ b/src/trio/_tests/test_channel.py @@ -11,7 +11,7 @@ from ..testing import Matcher, RaisesGroup, assert_checkpoints, wait_all_tasks_blocked if sys.version_info < (3, 11): - from exceptiongroup import ExceptionGroup + from exceptiongroup import BaseExceptionGroup, ExceptionGroup if TYPE_CHECKING: from collections.abc import AsyncGenerator @@ -625,3 +625,30 @@ async def agen(events: list[str]) -> AsyncGenerator[None]: events.append("body cancel") raise assert events == ["body cancel", "agen cancel"] + + +async def test_as_safe_channel_genexit_exception_group() -> None: + @as_safe_channel + async def agen() -> AsyncGenerator[None]: + try: + async with trio.open_nursery(): + yield + except BaseException as e: + assert isinstance(e, BaseExceptionGroup) # noqa: PT017 # we reraise + raise + + async with agen() as g: + async for _ in g: + break + + +async def test_as_safe_channel_does_not_suppress_nested_genexit() -> None: + @as_safe_channel + async def agen() -> AsyncGenerator[None]: + while True: + yield + + with pytest.RaisesGroup(GeneratorExit): + async with agen() as g, trio.open_nursery(): + async for _ in g: + raise GeneratorExit From 14bb557a8d89f86b6b3a205cf8e9cf3a15f50e8d Mon Sep 17 00:00:00 2001 From: A5rocks Date: Wed, 3 Sep 2025 23:03:56 +0900 Subject: [PATCH 2/8] Add a newsfragment --- newsfragments/3324.bugfix.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 newsfragments/3324.bugfix.rst diff --git a/newsfragments/3324.bugfix.rst b/newsfragments/3324.bugfix.rst new file mode 100644 index 0000000000..0fd5a05399 --- /dev/null +++ b/newsfragments/3324.bugfix.rst @@ -0,0 +1,2 @@ +Avoid having `trio.as_safe_channel` raise if closing the generator wrapped +`GeneratorExit` in a `BaseExceptionGroup`. From 1904b982535e9a4b5bbecf165d52fbe5dfb93caa Mon Sep 17 00:00:00 2001 From: A5rocks Date: Wed, 3 Sep 2025 23:39:47 +0900 Subject: [PATCH 3/8] Fix type issue --- src/trio/_channel.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/trio/_channel.py b/src/trio/_channel.py index 6f81d563b2..6405d90a4a 100644 --- a/src/trio/_channel.py +++ b/src/trio/_channel.py @@ -589,9 +589,9 @@ async def _move_elems_to_channel( # TODO: make an issue on CPython about this try: await agen.aclose() - except BaseExceptionGroup as eg: - _, eg = eg.split(GeneratorExit) - if eg is not None: - raise eg + except BaseExceptionGroup as exceptions: + _, narrowed_exceptions = exceptions.split(GeneratorExit) + if narrowed_exceptions is not None: + raise narrowed_exceptions from None return context_manager From 7993fac1f3699fa02185591a5360acaadafd7fbf Mon Sep 17 00:00:00 2001 From: A5rocks Date: Wed, 3 Sep 2025 23:54:44 +0900 Subject: [PATCH 4/8] Address coverage --- src/trio/_tests/test_channel.py | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/src/trio/_tests/test_channel.py b/src/trio/_tests/test_channel.py index 7c4e0b00a5..b878b5fb78 100644 --- a/src/trio/_tests/test_channel.py +++ b/src/trio/_tests/test_channel.py @@ -645,10 +645,27 @@ async def agen() -> AsyncGenerator[None]: async def test_as_safe_channel_does_not_suppress_nested_genexit() -> None: @as_safe_channel async def agen() -> AsyncGenerator[None]: - while True: - yield + yield with pytest.RaisesGroup(GeneratorExit): - async with agen() as g, trio.open_nursery(): + async with agen(), trio.open_nursery(): + raise GeneratorExit + + +async def test_as_safe_channel_genexit_filter() -> None: + async def wait_then_raise() -> None: + try: + await trio.sleep_forever() + except trio.Cancelled: + raise ValueError from None + + @as_safe_channel + async def agen() -> AsyncGenerator[None]: + async with trio.open_nursery() as nursery: + nursery.start_soon(wait_then_raise) + yield + + with pytest.RaisesGroup(ValueError): + async with agen() as g: async for _ in g: - raise GeneratorExit + break From b1fda4f805d021ead5fb815b52b44869e4bb57dc Mon Sep 17 00:00:00 2001 From: A5rocks Date: Thu, 4 Sep 2025 00:04:59 +0900 Subject: [PATCH 5/8] Try one more coverage workaround --- src/trio/_tests/test_channel.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/trio/_tests/test_channel.py b/src/trio/_tests/test_channel.py index b878b5fb78..7feefe0632 100644 --- a/src/trio/_tests/test_channel.py +++ b/src/trio/_tests/test_channel.py @@ -648,7 +648,8 @@ async def agen() -> AsyncGenerator[None]: yield with pytest.RaisesGroup(GeneratorExit): - async with agen(), trio.open_nursery(): + async with agen() as g, trio.open_nursery(): + await g.receive() # this is for coverage reasons raise GeneratorExit From d98b85e979cb983e956c5f83280ef3594439aa1f Mon Sep 17 00:00:00 2001 From: A5rocks Date: Fri, 5 Sep 2025 08:44:24 +0900 Subject: [PATCH 6/8] PR review --- src/trio/_channel.py | 26 +++++++++++++++++++++++++- src/trio/_tests/test_channel.py | 23 +++++++++++++++++++++-- 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/src/trio/_channel.py b/src/trio/_channel.py index 6405d90a4a..0112570017 100644 --- a/src/trio/_channel.py +++ b/src/trio/_channel.py @@ -590,7 +590,31 @@ async def _move_elems_to_channel( try: await agen.aclose() except BaseExceptionGroup as exceptions: - _, narrowed_exceptions = exceptions.split(GeneratorExit) + removed, narrowed_exceptions = exceptions.split(GeneratorExit) + + # TODO: extract a helper to flatten exception groups + removed_exceptions = [removed] + for e in removed_exceptions: + if isinstance(e, BaseExceptionGroup): + removed_exceptions.extend(e.exceptions) # noqa: B909 + + if ( + len( + [ + e + for e in removed_exceptions + if isinstance(e, GeneratorExit) + ] + ) + > 1 + ): + exc = AssertionError("More than one GeneratorExit found.") + if narrowed_exceptions is None: + narrowed_exceptions = exceptions.derive([exc]) + else: + narrowed_exceptions = narrowed_exceptions.derive( + [*narrowed_exceptions.exceptions, exc] + ) if narrowed_exceptions is not None: raise narrowed_exceptions from None diff --git a/src/trio/_tests/test_channel.py b/src/trio/_tests/test_channel.py index 7feefe0632..8ceb6a574c 100644 --- a/src/trio/_tests/test_channel.py +++ b/src/trio/_tests/test_channel.py @@ -11,7 +11,7 @@ from ..testing import Matcher, RaisesGroup, assert_checkpoints, wait_all_tasks_blocked if sys.version_info < (3, 11): - from exceptiongroup import BaseExceptionGroup, ExceptionGroup + from exceptiongroup import ExceptionGroup if TYPE_CHECKING: from collections.abc import AsyncGenerator @@ -634,7 +634,7 @@ async def agen() -> AsyncGenerator[None]: async with trio.open_nursery(): yield except BaseException as e: - assert isinstance(e, BaseExceptionGroup) # noqa: PT017 # we reraise + assert pytest.RaisesGroup(GeneratorExit).matches(e) # noqa: PT017 raise async with agen() as g: @@ -670,3 +670,22 @@ async def agen() -> AsyncGenerator[None]: async with agen() as g: async for _ in g: break + + +async def test_as_safe_channel_swallowing_extra_exceptions() -> None: + async def wait_then_raise() -> None: + try: + await trio.sleep_forever() + except trio.Cancelled: + raise GeneratorExit from None + + @as_safe_channel + async def agen() -> AsyncGenerator[None]: + async with trio.open_nursery() as nursery: + nursery.start_soon(wait_then_raise) + yield + + with pytest.RaisesGroup(AssertionError): + async with agen() as g: + async for _ in g: + break From ea761df140c12a2dea52e7340817333aa941fc80 Mon Sep 17 00:00:00 2001 From: A5rocks Date: Fri, 5 Sep 2025 09:14:39 +0900 Subject: [PATCH 7/8] Appease mypy + PR review --- src/trio/_channel.py | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/trio/_channel.py b/src/trio/_channel.py index 0112570017..2afca9d7cd 100644 --- a/src/trio/_channel.py +++ b/src/trio/_channel.py @@ -593,21 +593,15 @@ async def _move_elems_to_channel( removed, narrowed_exceptions = exceptions.split(GeneratorExit) # TODO: extract a helper to flatten exception groups - removed_exceptions = [removed] + removed_exceptions: list[BaseException | None] = [removed] + genexits_seen = 0 for e in removed_exceptions: if isinstance(e, BaseExceptionGroup): removed_exceptions.extend(e.exceptions) # noqa: B909 + else: + genexits_seen += 1 - if ( - len( - [ - e - for e in removed_exceptions - if isinstance(e, GeneratorExit) - ] - ) - > 1 - ): + if genexits_seen > 1: exc = AssertionError("More than one GeneratorExit found.") if narrowed_exceptions is None: narrowed_exceptions = exceptions.derive([exc]) From 1af562020593fdaef5d46c9d6d2c0f49d1119dc3 Mon Sep 17 00:00:00 2001 From: A5rocks Date: Sat, 6 Sep 2025 10:31:45 +0900 Subject: [PATCH 8/8] Address coverage --- src/trio/_tests/test_channel.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/trio/_tests/test_channel.py b/src/trio/_tests/test_channel.py index 8ceb6a574c..85cd982a41 100644 --- a/src/trio/_tests/test_channel.py +++ b/src/trio/_tests/test_channel.py @@ -673,19 +673,25 @@ async def agen() -> AsyncGenerator[None]: async def test_as_safe_channel_swallowing_extra_exceptions() -> None: - async def wait_then_raise() -> None: + async def wait_then_raise(ex: type[BaseException]) -> None: try: await trio.sleep_forever() except trio.Cancelled: - raise GeneratorExit from None + raise ex from None @as_safe_channel - async def agen() -> AsyncGenerator[None]: + async def agen(ex: type[BaseException]) -> AsyncGenerator[None]: async with trio.open_nursery() as nursery: - nursery.start_soon(wait_then_raise) + nursery.start_soon(wait_then_raise, ex) + nursery.start_soon(wait_then_raise, GeneratorExit) yield with pytest.RaisesGroup(AssertionError): - async with agen() as g: + async with agen(GeneratorExit) as g: + async for _ in g: + break + + with pytest.RaisesGroup(ValueError, AssertionError): + async with agen(ValueError) as g: async for _ in g: break