diff --git a/newsfragments/3232.feature.rst b/newsfragments/3232.feature.rst new file mode 100644 index 0000000000..9da76cb370 --- /dev/null +++ b/newsfragments/3232.feature.rst @@ -0,0 +1 @@ +:exc:`Cancelled` strings can now display the source and reason for a cancellation. Trio-internal sources of cancellation will set this string, and :meth:`CancelScope.cancel` now has a ``reason`` string parameter that can be used to attach info to any :exc:`Cancelled` to help in debugging. diff --git a/src/trio/_channel.py b/src/trio/_channel.py index 1ed5945798..54b5ea4bea 100644 --- a/src/trio/_channel.py +++ b/src/trio/_channel.py @@ -547,7 +547,9 @@ async def context_manager( yield wrapped_recv_chan # User has exited context manager, cancel to immediately close the # abandoned generator if it's still alive. - nursery.cancel_scope.cancel() + nursery.cancel_scope.cancel( + "exited trio.as_safe_channel context manager" + ) except BaseExceptionGroup as eg: try: raise_single_exception_from_group(eg) diff --git a/src/trio/_core/_asyncgens.py b/src/trio/_core/_asyncgens.py index b3b6895753..fea41e0e4d 100644 --- a/src/trio/_core/_asyncgens.py +++ b/src/trio/_core/_asyncgens.py @@ -230,7 +230,9 @@ async def _finalize_one( # with an exception, not even a Cancelled. The inside # is cancelled so there's no deadlock risk. with _core.CancelScope(shield=True) as cancel_scope: - cancel_scope.cancel() + cancel_scope.cancel( + reason="disallow async work when closing async generators during trio shutdown" + ) await agen.aclose() except BaseException: ASYNCGEN_LOGGER.exception( diff --git a/src/trio/_core/_exceptions.py b/src/trio/_core/_exceptions.py index 4a76a674ac..0974d8eb5e 100644 --- a/src/trio/_core/_exceptions.py +++ b/src/trio/_core/_exceptions.py @@ -1,12 +1,26 @@ from __future__ import annotations -from typing import TYPE_CHECKING +from functools import partial +from typing import TYPE_CHECKING, Literal + +import attrs from trio._util import NoPublicConstructor, final if TYPE_CHECKING: from collections.abc import Callable + from typing_extensions import Self, TypeAlias + +CancelReasonLiteral: TypeAlias = Literal[ + "KeyboardInterrupt", + "deadline", + "explicit", + "nursery", + "shutdown", + "unknown", +] + class TrioInternalError(Exception): """Raised by :func:`run` if we encounter a bug in Trio, or (possibly) a @@ -34,6 +48,7 @@ class WouldBlock(Exception): @final +@attrs.define(eq=False, kw_only=True) class Cancelled(BaseException, metaclass=NoPublicConstructor): """Raised by blocking calls if the surrounding scope has been cancelled. @@ -67,11 +82,42 @@ class Cancelled(BaseException, metaclass=NoPublicConstructor): """ + source: CancelReasonLiteral + # repr(Task), so as to avoid gc troubles from holding a reference + source_task: str | None = None + reason: str | None = None + def __str__(self) -> str: - return "Cancelled" + return ( + f"cancelled due to {self.source}" + + ("" if self.reason is None else f" with reason {self.reason!r}") + + ("" if self.source_task is None else f" from task {self.source_task}") + ) def __reduce__(self) -> tuple[Callable[[], Cancelled], tuple[()]]: - return (Cancelled._create, ()) + # The `__reduce__` tuple does not support directly passing kwargs, and the + # kwargs are required so we can't use the third item for adding to __dict__, + # so we use partial. + return ( + partial( + Cancelled._create, + source=self.source, + source_task=self.source_task, + reason=self.reason, + ), + (), + ) + + if TYPE_CHECKING: + # for type checking on internal code + @classmethod + def _create( + cls, + *, + source: CancelReasonLiteral, + source_task: str | None = None, + reason: str | None = None, + ) -> Self: ... class BusyResourceError(Exception): diff --git a/src/trio/_core/_run.py b/src/trio/_core/_run.py index c2410150e1..5fff0b772f 100644 --- a/src/trio/_core/_run.py +++ b/src/trio/_core/_run.py @@ -36,7 +36,12 @@ from ._asyncgens import AsyncGenerators from ._concat_tb import concat_tb from ._entry_queue import EntryQueue, TrioToken -from ._exceptions import Cancelled, RunFinishedError, TrioInternalError +from ._exceptions import ( + Cancelled, + CancelReasonLiteral, + RunFinishedError, + TrioInternalError, +) from ._instrumentation import Instruments from ._ki import KIManager, enable_ki_protection from ._parking_lot import GLOBAL_PARKING_LOT_BREAKER @@ -305,7 +310,7 @@ def expire(self, now: float) -> bool: did_something = True # This implicitly calls self.remove(), so we don't need to # decrement _active here - cancel_scope.cancel() + cancel_scope._cancel(CancelReason(source="deadline")) # If we've accumulated too many stale entries, then prune the heap to # keep it under control. (We only do this occasionally in a batch, to # keep the amortized cost down) @@ -314,6 +319,20 @@ def expire(self, now: float) -> bool: return did_something +@attrs.define +class CancelReason: + """Attached to a :class:`CancelScope` upon cancellation with details of the source of the + cancellation, which is then used to construct the string in a :exc:`Cancelled`. + Users can pass a ``reason`` str to :meth:`CancelScope.cancel` to set it. + + Not publicly exported or documented. + """ + + source: CancelReasonLiteral + source_task: str | None = None + reason: str | None = None + + @attrs.define(eq=False) class CancelStatus: """Tracks the cancellation status for a contiguous extent @@ -468,6 +487,14 @@ def recalculate(self) -> None: or current.parent_cancellation_is_visible_to_us ) if new_state != current.effectively_cancelled: + if ( + current._scope._cancel_reason is None + and current.parent_cancellation_is_visible_to_us + ): + assert current._parent is not None + current._scope._cancel_reason = ( + current._parent._scope._cancel_reason + ) current.effectively_cancelled = new_state if new_state: for task in current._tasks: @@ -558,6 +585,8 @@ class CancelScope: _cancel_called: bool = attrs.field(default=False, init=False) cancelled_caught: bool = attrs.field(default=False, init=False) + _cancel_reason: CancelReason | None = attrs.field(default=None, init=False) + # Constructor arguments: _relative_deadline: float = attrs.field( default=inf, @@ -594,7 +623,7 @@ def __enter__(self) -> Self: self._relative_deadline = inf if current_time() >= self._deadline: - self.cancel() + self._cancel(CancelReason(source="deadline")) with self._might_change_registered_deadline(): self._cancel_status = CancelStatus(scope=self, parent=task._cancel_status) task._activate_cancel_status(self._cancel_status) @@ -883,19 +912,42 @@ def shield(self, new_value: bool) -> None: self._cancel_status.recalculate() @enable_ki_protection - def cancel(self) -> None: - """Cancels this scope immediately. - - This method is idempotent, i.e., if the scope was already - cancelled then this method silently does nothing. + def _cancel(self, cancel_reason: CancelReason | None) -> None: + """Internal sources of cancellation should use this instead of :meth:`cancel` + in order to set a more detailed :class:`CancelReason` + Helper or high-level functions can use `cancel`. """ if self._cancel_called: return + + if self._cancel_reason is None: + self._cancel_reason = cancel_reason + with self._might_change_registered_deadline(): self._cancel_called = True + if self._cancel_status is not None: self._cancel_status.recalculate() + @enable_ki_protection + def cancel(self, reason: str | None = None) -> None: + """Cancels this scope immediately. + + The optional ``reason`` argument accepts a string, which will be attached to + any resulting :exc:`Cancelled` exception to help you understand where that + cancellation is coming from and why it happened. + + This method is idempotent, i.e., if the scope was already + cancelled then this method silently does nothing. + """ + try: + current_task = repr(_core.current_task()) + except RuntimeError: + current_task = None + self._cancel( + CancelReason(reason=reason, source="explicit", source_task=current_task) + ) + @property def cancel_called(self) -> bool: """Readonly :class:`bool`. Records whether cancellation has been @@ -924,7 +976,7 @@ def cancel_called(self) -> bool: # but it makes the value returned by cancel_called more # closely match expectations. if not self._cancel_called and current_time() >= self._deadline: - self.cancel() + self._cancel(CancelReason(source="deadline")) return self._cancel_called @@ -1192,9 +1244,9 @@ def parent_task(self) -> Task: "(`~trio.lowlevel.Task`): The Task that opened this nursery." return self._parent_task - def _add_exc(self, exc: BaseException) -> None: + def _add_exc(self, exc: BaseException, reason: CancelReason | None) -> None: self._pending_excs.append(exc) - self.cancel_scope.cancel() + self.cancel_scope._cancel(reason) def _check_nursery_closed(self) -> None: if not any([self._nested_child_running, self._children, self._pending_starts]): @@ -1210,7 +1262,14 @@ def _child_finished( ) -> None: self._children.remove(task) if isinstance(outcome, Error): - self._add_exc(outcome.error) + self._add_exc( + outcome.error, + CancelReason( + source="nursery", + source_task=repr(task), + reason=f"child task raised exception {outcome.error!r}", + ), + ) self._check_nursery_closed() async def _nested_child_finished( @@ -1220,7 +1279,14 @@ async def _nested_child_finished( # Returns ExceptionGroup instance (or any exception if the nursery is in loose mode # and there is just one contained exception) if there are pending exceptions if nested_child_exc is not None: - self._add_exc(nested_child_exc) + self._add_exc( + nested_child_exc, + reason=CancelReason( + source="nursery", + source_task=repr(self._parent_task), + reason=f"Code block inside nursery contextmanager raised exception {nested_child_exc!r}", + ), + ) self._nested_child_running = False self._check_nursery_closed() @@ -1231,7 +1297,13 @@ async def _nested_child_finished( def aborted(raise_cancel: _core.RaiseCancelT) -> Abort: exn = capture(raise_cancel).error if not isinstance(exn, Cancelled): - self._add_exc(exn) + self._add_exc( + exn, + CancelReason( + source="KeyboardInterrupt", + source_task=repr(self._parent_task), + ), + ) # see test_cancel_scope_exit_doesnt_create_cyclic_garbage del exn # prevent cyclic garbage creation return Abort.FAILED @@ -1245,7 +1317,8 @@ def aborted(raise_cancel: _core.RaiseCancelT) -> Abort: try: await cancel_shielded_checkpoint() except BaseException as exc: - self._add_exc(exc) + # there's no children to cancel, so don't need to supply cancel reason + self._add_exc(exc, reason=None) popped = self._parent_task._child_nurseries.pop() assert popped is self @@ -1575,8 +1648,17 @@ def _attempt_delivery_of_any_pending_cancel(self) -> None: if not self._cancel_status.effectively_cancelled: return + reason = self._cancel_status._scope._cancel_reason + def raise_cancel() -> NoReturn: - raise Cancelled._create() + if reason is None: + raise Cancelled._create(source="unknown", reason="misnesting") + else: + raise Cancelled._create( + source=reason.source, + reason=reason.reason, + source_task=reason.source_task, + ) self._attempt_abort(raise_cancel) @@ -2075,7 +2157,13 @@ async def init( ) # Main task is done; start shutting down system tasks - self.system_nursery.cancel_scope.cancel() + self.system_nursery.cancel_scope._cancel( + CancelReason( + source="shutdown", + reason="main task done, shutting down system tasks", + source_task=repr(self.init_task), + ) + ) # System nursery is closed; finalize remaining async generators await self.asyncgens.finalize_remaining(self) @@ -2083,7 +2171,13 @@ async def init( # There are no more asyncgens, which means no more user-provided # code except possibly run_sync_soon callbacks. It's finally safe # to stop the run_sync_soon task and exit run(). - run_sync_soon_nursery.cancel_scope.cancel() + run_sync_soon_nursery.cancel_scope._cancel( + CancelReason( + source="shutdown", + reason="main task done, shutting down run_sync_soon callbacks", + source_task=repr(self.init_task), + ) + ) ################ # Outside context problems @@ -2926,7 +3020,18 @@ async def checkpoint() -> None: if task._cancel_status.effectively_cancelled or ( task is task._runner.main_task and task._runner.ki_pending ): - with CancelScope(deadline=-inf): + cs = CancelScope(deadline=-inf) + if ( + task._cancel_status._scope._cancel_reason is None + and task is task._runner.main_task + and task._runner.ki_pending + ): + task._cancel_status._scope._cancel_reason = CancelReason( + source="KeyboardInterrupt" + ) + assert task._cancel_status._scope._cancel_reason is not None + cs._cancel_reason = task._cancel_status._scope._cancel_reason + with cs: await _core.wait_task_rescheduled(lambda _: _core.Abort.SUCCEEDED) diff --git a/src/trio/_core/_tests/test_cancelled.py b/src/trio/_core/_tests/test_cancelled.py new file mode 100644 index 0000000000..18af4a367e --- /dev/null +++ b/src/trio/_core/_tests/test_cancelled.py @@ -0,0 +1,222 @@ +import pickle +import re +from math import inf + +import pytest + +import trio +from trio import Cancelled +from trio.lowlevel import current_task +from trio.testing import RaisesGroup, wait_all_tasks_blocked + +from .test_ki import ki_self + + +def test_Cancelled_init() -> None: + with pytest.raises(TypeError, match=r"^trio.Cancelled has no public constructor$"): + raise Cancelled # type: ignore[call-arg] + + with pytest.raises(TypeError, match=r"^trio.Cancelled has no public constructor$"): + Cancelled(source="explicit") + + # private constructor should not raise + Cancelled._create(source="explicit") + + +async def test_Cancelled_str() -> None: + cancelled = Cancelled._create(source="explicit") + assert str(cancelled) == "cancelled due to explicit" + # note: repr(current_task()) is often fairly verbose + assert re.fullmatch( + r"cancelled due to deadline from task " + r"", + str( + Cancelled._create( + source="deadline", + source_task=repr(current_task()), + ) + ), + ) + + assert re.fullmatch( + rf"cancelled due to nursery with reason 'pigs flying' from task {current_task()!r}", + str( + Cancelled._create( + source="nursery", + source_task=repr(current_task()), + reason="pigs flying", + ) + ), + ) + + +def test_Cancelled_subclass() -> None: + with pytest.raises(TypeError): + type("Subclass", (Cancelled,), {}) + + +# https://github.com/python-trio/trio/issues/3248 +def test_Cancelled_pickle() -> None: + cancelled = Cancelled._create(source="KeyboardInterrupt") + pickled_cancelled = pickle.loads(pickle.dumps(cancelled)) + assert isinstance(pickled_cancelled, Cancelled) + assert cancelled.source == pickled_cancelled.source + assert cancelled.source_task == pickled_cancelled.source_task + assert cancelled.reason == pickled_cancelled.reason + + +async def test_cancel_reason() -> None: + with trio.CancelScope() as cs: + cs.cancel(reason="hello") + with pytest.raises( + Cancelled, + match=rf"^cancelled due to explicit with reason 'hello' from task {current_task()!r}$", + ) as excinfo: + await trio.lowlevel.checkpoint() + assert excinfo.value.source == "explicit" + assert excinfo.value.reason == "hello" + assert excinfo.value.source_task == repr(current_task()) + + with trio.CancelScope(deadline=-inf) as cs: + with pytest.raises(Cancelled, match=r"^cancelled due to deadline"): + await trio.lowlevel.checkpoint() + + with trio.CancelScope() as cs: + cs.deadline = -inf + with pytest.raises( + Cancelled, + match=r"^cancelled due to deadline", + ): + await trio.lowlevel.checkpoint() + + +match_str = r"^cancelled due to nursery with reason 'child task raised exception ValueError\(\)' from task {0!r}$" + + +async def cancelled_task( + fail_task: trio.lowlevel.Task, task_status: trio.TaskStatus +) -> None: + task_status.started() + with pytest.raises(Cancelled, match=match_str.format(fail_task)): + await trio.sleep_forever() + raise TypeError + + +# failing_task raises before cancelled_task is started +async def test_cancel_reason_nursery() -> None: + async def failing_task(task_status: trio.TaskStatus[trio.lowlevel.Task]) -> None: + task_status.started(current_task()) + raise ValueError + + with RaisesGroup(ValueError, TypeError): + async with trio.open_nursery() as nursery: + fail_task = await nursery.start(failing_task) + with pytest.raises(Cancelled, match=match_str.format(fail_task)): + await wait_all_tasks_blocked() + await nursery.start(cancelled_task, fail_task) + + +# wait until both tasks are running before failing_task raises +async def test_cancel_reason_nursery2() -> None: + async def failing_task(task_status: trio.TaskStatus[trio.lowlevel.Task]) -> None: + task_status.started(current_task()) + await wait_all_tasks_blocked() + raise ValueError + + with RaisesGroup(ValueError, TypeError): + async with trio.open_nursery() as nursery: + fail_task = await nursery.start(failing_task) + await nursery.start(cancelled_task, fail_task) + + +# failing_task raises before calling task_status.started() +async def test_cancel_reason_nursery3() -> None: + async def failing_task(task_status: trio.TaskStatus[None]) -> None: + raise ValueError + + parent_task = current_task() + + async def cancelled_task() -> None: + # We don't have a way of distinguishing that the nursery code block failed + # because it failed to `start()` a task. + with pytest.raises( + Cancelled, + match=re.escape( + rf"cancelled due to nursery with reason 'Code block inside nursery contextmanager raised exception ValueError()' from task {parent_task!r}" + ), + ): + await trio.sleep_forever() + + with RaisesGroup(ValueError): + async with trio.open_nursery() as nursery: + nursery.start_soon(cancelled_task) + await wait_all_tasks_blocked() + await nursery.start(failing_task) + + +async def test_cancel_reason_not_overwritten() -> None: + with trio.CancelScope() as cs: + cs.cancel() + with pytest.raises( + Cancelled, + match=rf"^cancelled due to explicit from task {current_task()!r}$", + ): + await trio.lowlevel.checkpoint() + cs.deadline = -inf + with pytest.raises( + Cancelled, + match=rf"^cancelled due to explicit from task {current_task()!r}$", + ): + await trio.lowlevel.checkpoint() + + +async def test_cancel_reason_not_overwritten_2() -> None: + with trio.CancelScope() as cs: + cs.deadline = -inf + with pytest.raises(Cancelled, match=r"^cancelled due to deadline$"): + await trio.lowlevel.checkpoint() + cs.cancel() + with pytest.raises(Cancelled, match=r"^cancelled due to deadline$"): + await trio.lowlevel.checkpoint() + + +async def test_nested_child_source() -> None: + ev = trio.Event() + parent_task = current_task() + + async def child() -> None: + ev.set() + with pytest.raises( + Cancelled, + match=rf"^cancelled due to nursery with reason 'Code block inside nursery contextmanager raised exception ValueError\(\)' from task {parent_task!r}$", + ): + await trio.sleep_forever() + + with RaisesGroup(ValueError): + async with trio.open_nursery() as nursery: + nursery.start_soon(child) + await ev.wait() + raise ValueError + + +async def test_reason_delayed_ki() -> None: + # simplified version of test_ki.test_ki_protection_works check #2 + parent_task = current_task() + + async def sleeper(name: str) -> None: + with pytest.raises( + Cancelled, + match=rf"^cancelled due to KeyboardInterrupt from task {parent_task!r}$", + ): + while True: + await trio.lowlevel.checkpoint() + + async def raiser(name: str) -> None: + ki_self() + + with RaisesGroup(KeyboardInterrupt): + async with trio.open_nursery() as nursery: + nursery.start_soon(sleeper, "s1") + nursery.start_soon(sleeper, "s2") + nursery.start_soon(trio.lowlevel.enable_ki_protection(raiser), "r1") + # __aexit__ blocks, and then receives the KI diff --git a/src/trio/_core/_tests/test_run.py b/src/trio/_core/_tests/test_run.py index f3e53ace38..5026c139c6 100644 --- a/src/trio/_core/_tests/test_run.py +++ b/src/trio/_core/_tests/test_run.py @@ -3,7 +3,6 @@ import contextvars import functools import gc -import pickle import sys import threading import time @@ -781,7 +780,10 @@ async def task1() -> None: # Even if inside another cancel scope async def task2() -> None: with _core.CancelScope(): - with pytest.raises(_core.Cancelled): + with pytest.raises( + _core.Cancelled, + match=r"^cancelled due to unknown with reason 'misnesting'$", + ): await sleep_forever() with ExitStack() as stack: @@ -2221,34 +2223,6 @@ def test_Nursery_subclass() -> None: type("Subclass", (_core._run.Nursery,), {}) -def test_Cancelled_init() -> None: - with pytest.raises(TypeError): - raise _core.Cancelled - - with pytest.raises(TypeError): - _core.Cancelled() - - # private constructor should not raise - _core.Cancelled._create() - - -def test_Cancelled_str() -> None: - cancelled = _core.Cancelled._create() - assert str(cancelled) == "Cancelled" - - -def test_Cancelled_subclass() -> None: - with pytest.raises(TypeError): - type("Subclass", (_core.Cancelled,), {}) - - -# https://github.com/python-trio/trio/issues/3248 -def test_Cancelled_pickle() -> None: - cancelled = _core.Cancelled._create() - cancelled = pickle.loads(pickle.dumps(cancelled)) - assert isinstance(cancelled, _core.Cancelled) - - def test_CancelScope_subclass() -> None: with pytest.raises(TypeError): type("Subclass", (_core.CancelScope,), {}) diff --git a/src/trio/_highlevel_generic.py b/src/trio/_highlevel_generic.py index 041a684c62..9bd8822c9e 100644 --- a/src/trio/_highlevel_generic.py +++ b/src/trio/_highlevel_generic.py @@ -43,7 +43,7 @@ async def aclose_forcefully(resource: AsyncResource) -> None: """ with trio.CancelScope() as cs: - cs.cancel() + cs.cancel(reason="cancelled during aclose_forcefully") await resource.aclose() diff --git a/src/trio/_highlevel_open_tcp_stream.py b/src/trio/_highlevel_open_tcp_stream.py index 5723180e46..11460689b4 100644 --- a/src/trio/_highlevel_open_tcp_stream.py +++ b/src/trio/_highlevel_open_tcp_stream.py @@ -357,7 +357,7 @@ async def attempt_connect( # Success! Save the winning socket and cancel all outstanding # connection attempts. winning_socket = sock - nursery.cancel_scope.cancel() + nursery.cancel_scope.cancel(reason="successfully found a socket") except OSError as exc: # This connection attempt failed, but the next one might # succeed. Save the error for later so we can report it if diff --git a/src/trio/_subprocess.py b/src/trio/_subprocess.py index 823c50ea63..c1b162e308 100644 --- a/src/trio/_subprocess.py +++ b/src/trio/_subprocess.py @@ -766,7 +766,7 @@ async def killer() -> None: nursery.start_soon(killer) await proc.wait() - killer_cscope.cancel() + killer_cscope.cancel(reason="trio internal implementation detail") raise stdout = b"".join(stdout_chunks) if capture_stdout else None diff --git a/src/trio/_tests/test_threads.py b/src/trio/_tests/test_threads.py index 380da3833b..ba2c366faa 100644 --- a/src/trio/_tests/test_threads.py +++ b/src/trio/_tests/test_threads.py @@ -993,82 +993,113 @@ async def async_time_bomb() -> None: assert cancel_scope.cancelled_caught -async def test_from_thread_check_cancelled() -> None: - q: stdlib_queue.Queue[str] = stdlib_queue.Queue() +async def child( + abandon_on_cancel: bool, + scope: CancelScope, + record: list[str], + f: Callable[[], None], +) -> None: + with scope: + record.append("start") + try: + return await to_thread_run_sync(f, abandon_on_cancel=abandon_on_cancel) + except _core.Cancelled as e: + record.append(str(e)) + raise + finally: + record.append("exit") - async def child(abandon_on_cancel: bool, scope: CancelScope) -> None: - with scope: - record.append("start") - try: - return await to_thread_run_sync(f, abandon_on_cancel=abandon_on_cancel) - except _core.Cancelled: - record.append("cancel") - raise - finally: - record.append("exit") + +@pytest.mark.parametrize("cancel_the_scope", [False, True]) +async def test_from_thread_check_cancelled_no_abandon(cancel_the_scope: bool) -> None: + q: stdlib_queue.Queue[str | BaseException] = stdlib_queue.Queue() def f() -> None: try: from_thread_check_cancelled() - except _core.Cancelled: # pragma: no cover, test failure path - q.put("Cancelled") + except _core.Cancelled as e: # pragma: no cover, test failure path + q.put(str(e)) else: q.put("Not Cancelled") ev.wait() return from_thread_check_cancelled() - # Base case: nothing cancelled so we shouldn't see cancels anywhere record: list[str] = [] ev = threading.Event() - async with _core.open_nursery() as nursery: - nursery.start_soon(child, False, _core.CancelScope()) - await wait_all_tasks_blocked() - assert record[0] == "start" - assert q.get(timeout=1) == "Not Cancelled" - ev.set() - # implicit assertion, Cancelled not raised via nursery - assert record[1] == "exit" - - # abandon_on_cancel=False case: a cancel will pop out but be handled by - # the appropriate cancel scope - record = [] - ev = threading.Event() scope = _core.CancelScope() # Nursery cancel scope gives false positives + async with _core.open_nursery() as nursery: - nursery.start_soon(child, False, scope) + nursery.start_soon(child, False, scope, record, f) await wait_all_tasks_blocked() assert record[0] == "start" assert q.get(timeout=1) == "Not Cancelled" - scope.cancel() + if cancel_the_scope: + scope.cancel() ev.set() - assert scope.cancelled_caught - assert "cancel" in record - assert record[-1] == "exit" + # Base case: nothing cancelled so we shouldn't see cancels anywhere + if not cancel_the_scope: + # implicit assertion, Cancelled not raised via nursery + assert record[1] == "exit" + else: + # abandon_on_cancel=False case: a cancel will pop out but be handled by + # the appropriate cancel scope + + assert scope.cancelled_caught + assert re.fullmatch( + r"cancelled due to explicit from task " + r"", + record[1], + ), record[1] + assert record[2] == "exit" + assert len(record) == 3 + +async def test_from_thread_check_cancelled_abandon_on_cancel() -> None: + q: stdlib_queue.Queue[str | BaseException] = stdlib_queue.Queue() # abandon_on_cancel=True case: slightly different thread behavior needed # check thread is cancelled "soon" after abandonment - def f() -> None: # type: ignore[no-redef] # noqa: F811 + + def f() -> None: ev.wait() try: from_thread_check_cancelled() - except _core.Cancelled: - q.put("Cancelled") + except _core.Cancelled as e: + q.put(str(e)) + except BaseException as e: # pragma: no cover, test failure path + # abandon_on_cancel=True will eat exceptions, so we pass it + # through the queue in order to be able to debug any exceptions + q.put(e) else: # pragma: no cover, test failure path q.put("Not Cancelled") - record = [] + record: list[str] = [] ev = threading.Event() scope = _core.CancelScope() async with _core.open_nursery() as nursery: - nursery.start_soon(child, True, scope) + nursery.start_soon(child, True, scope, record, f) await wait_all_tasks_blocked() assert record[0] == "start" scope.cancel() - ev.set() + # In the worst case the nursery fully exits before the threaded function + # checks for cancellation. + ev.set() + assert scope.cancelled_caught - assert "cancel" in record + assert re.fullmatch( + r"cancelled due to explicit from task " + r"", + record[1], + ), record[1] assert record[-1] == "exit" - assert q.get(timeout=1) == "Cancelled" + res = q.get(timeout=1) + if isinstance(res, BaseException): # pragma: no cover # for debugging + raise res + else: + assert re.fullmatch( + r"cancelled due to explicit from task " + r"", + res, + ), res def test_from_thread_check_cancelled_raises_in_foreign_threads() -> None: diff --git a/src/trio/_tests/test_util.py b/src/trio/_tests/test_util.py index c0b0a3108e..4182ba5db4 100644 --- a/src/trio/_tests/test_util.py +++ b/src/trio/_tests/test_util.py @@ -282,7 +282,7 @@ async def test_raise_single_exception_from_group() -> None: context = TypeError("context") exc.__cause__ = cause exc.__context__ = context - cancelled = trio.Cancelled._create() + cancelled = trio.Cancelled._create(source="deadline") with pytest.raises(ValueError, match="foo") as excinfo: raise_single_exception_from_group(ExceptionGroup("", [exc])) @@ -346,9 +346,11 @@ async def test_raise_single_exception_from_group() -> None: assert excinfo.value.__context__ is None # if we only got cancelled, first one is reraised - with pytest.raises(trio.Cancelled, match=r"^Cancelled$") as excinfo: + with pytest.raises(trio.Cancelled, match=r"^cancelled due to deadline$") as excinfo: raise_single_exception_from_group( - BaseExceptionGroup("", [cancelled, trio.Cancelled._create()]) + BaseExceptionGroup( + "", [cancelled, trio.Cancelled._create(source="explicit")] + ) ) assert excinfo.value is cancelled assert excinfo.value.__cause__ is None diff --git a/src/trio/_threads.py b/src/trio/_threads.py index 394e5b06ac..4b1e54f540 100644 --- a/src/trio/_threads.py +++ b/src/trio/_threads.py @@ -430,6 +430,8 @@ def deliver_worker_fn_result(result: outcome.Outcome[RetT]) -> None: def abort(raise_cancel: RaiseCancelT) -> trio.lowlevel.Abort: # fill so from_thread_check_cancelled can raise + # 'raise_cancel' will immediately delete its reason object, so we make + # a copy in each thread cancel_register[0] = raise_cancel if abandon_bool: # empty so report_back_in_trio_thread_fn cannot reschedule diff --git a/src/trio/_util.py b/src/trio/_util.py index 6656749111..106423e2aa 100644 --- a/src/trio/_util.py +++ b/src/trio/_util.py @@ -29,12 +29,11 @@ import sys from types import AsyncGeneratorType, TracebackType - from typing_extensions import ParamSpec, Self, TypeVarTuple, Unpack + from typing_extensions import Self, TypeVarTuple, Unpack if sys.version_info < (3, 11): from exceptiongroup import BaseExceptionGroup - ArgsT = ParamSpec("ArgsT") PosArgsT = TypeVarTuple("PosArgsT")