From 737e5bb475d276a362441047d582ff4039cb7a10 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Mon, 29 Jan 2024 15:53:59 +0100 Subject: [PATCH 1/2] Replace usage of strict_exception_groups=False in Nursery.start --- src/trio/_core/_run.py | 36 ++++++++++++++++++++----------- src/trio/_core/_tests/test_run.py | 16 ++++++++++++++ 2 files changed, 39 insertions(+), 13 deletions(-) diff --git a/src/trio/_core/_run.py b/src/trio/_core/_run.py index 7282883723..2c1e9f2554 100644 --- a/src/trio/_core/_run.py +++ b/src/trio/_core/_run.py @@ -1231,19 +1231,29 @@ async def async_fn(arg1, arg2, *, task_status=trio.TASK_STATUS_IGNORED): raise RuntimeError("Nursery is closed to new arrivals") try: self._pending_starts += 1 - # `strict_exception_groups=False` prevents the implementation-detail - # nursery from inheriting `strict_exception_groups=True` from the - # `run` option, which would cause it to wrap a pre-started() - # exception in an extra ExceptionGroup. See #2611. - async with open_nursery(strict_exception_groups=False) as old_nursery: - task_status: _TaskStatus[Any] = _TaskStatus(old_nursery, self) - thunk = functools.partial(async_fn, task_status=task_status) - task = GLOBAL_RUN_CONTEXT.runner.spawn_impl( - thunk, args, old_nursery, name - ) - task._eventual_parent_nursery = self - # Wait for either TaskStatus.started or an exception to - # cancel this nursery: + # wrap internal nursery in try-except to unroll any exceptiongroups + # to avoid wrapping pre-started() exceptions in an extra ExceptionGroup. + # See #2611. + try: + # set strict_exception_groups = True to make sure we always unwrap + # *this* nursery's exceptiongroup + async with open_nursery(strict_exception_groups=True) as old_nursery: + task_status: _TaskStatus[Any] = _TaskStatus(old_nursery, self) + thunk = functools.partial(async_fn, task_status=task_status) + task = GLOBAL_RUN_CONTEXT.runner.spawn_impl( + thunk, args, old_nursery, name + ) + task._eventual_parent_nursery = self + # Wait for either TaskStatus.started or an exception to + # cancel this nursery: + except BaseExceptionGroup as exc: + if len(exc.exceptions) == 1: + raise exc.exceptions[0] from None + # TODO: give a deprecationwarning instead? + raise TrioInternalError( + "internal nursery should not have multiple tasks" + ) from exc + # If we get here, then the child either got reparented or exited # normally. The complicated logic is all in TaskStatus.started(). # (Any exceptions propagate directly out of the above.) diff --git a/src/trio/_core/_tests/test_run.py b/src/trio/_core/_tests/test_run.py index adfee663cb..b1011a4e1b 100644 --- a/src/trio/_core/_tests/test_run.py +++ b/src/trio/_core/_tests/test_run.py @@ -2700,3 +2700,19 @@ async def start_raiser() -> None: assert type(should_be_raiser_exc) == type(raiser_exc) assert should_be_raiser_exc.message == raiser_exc.message assert should_be_raiser_exc.exceptions == raiser_exc.exceptions + + +async def test_internal_error_old_nursery_multiple_tasks() -> None: + async def error_func() -> None: + raise ValueError + + async def spawn_tasks_in_old_nursery(task_status: _core.TaskStatus[None]) -> None: + old_nursery = _core.current_task().parent_nursery + assert old_nursery is not None + old_nursery.start_soon(error_func) + old_nursery.start_soon(error_func) + + async with _core.open_nursery() as nursery: + with pytest.raises(_core.TrioInternalError) as excinfo: + await nursery.start(spawn_tasks_in_old_nursery) + assert RaisesGroup(ValueError, ValueError).matches(excinfo.value.__cause__) From 0ce52c1cdbc10e544b7fdcf250cae3eff3505068 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Tue, 30 Jan 2024 15:06:31 +0100 Subject: [PATCH 2/2] remove TODO comment, expand error message --- src/trio/_core/_run.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/trio/_core/_run.py b/src/trio/_core/_run.py index 2c1e9f2554..d7e2d0474c 100644 --- a/src/trio/_core/_run.py +++ b/src/trio/_core/_run.py @@ -1249,9 +1249,10 @@ async def async_fn(arg1, arg2, *, task_status=trio.TASK_STATUS_IGNORED): except BaseExceptionGroup as exc: if len(exc.exceptions) == 1: raise exc.exceptions[0] from None - # TODO: give a deprecationwarning instead? raise TrioInternalError( - "internal nursery should not have multiple tasks" + "Internal nursery should not have multiple tasks. This can be " + 'caused by the user managing to access the "old" nursery in ' + "`task_status` and spawning tasks in it." ) from exc # If we get here, then the child either got reparented or exited