diff --git a/docs/source/conf.py b/docs/source/conf.py index ecdbdd4701..2e84d91532 100755 --- a/docs/source/conf.py +++ b/docs/source/conf.py @@ -229,6 +229,7 @@ def setup(app: Sphinx) -> None: "pyopenssl": ("https://www.pyopenssl.org/en/stable/", None), "sniffio": ("https://sniffio.readthedocs.io/en/latest/", None), "trio-util": ("https://trio-util.readthedocs.io/en/latest/", None), + "flake8-async": ("https://flake8-async.readthedocs.io/en/latest/", None), } # See https://sphinx-hoverxref.readthedocs.io/en/latest/configuration.html diff --git a/docs/source/reference-core.rst b/docs/source/reference-core.rst index 392eac7d20..4b023292e9 100644 --- a/docs/source/reference-core.rst +++ b/docs/source/reference-core.rst @@ -527,8 +527,12 @@ objects. .. autoattribute:: deadline + .. autoattribute:: relative_deadline + .. autoattribute:: shield + .. automethod:: is_relative() + .. automethod:: cancel() .. attribute:: cancelled_caught @@ -561,7 +565,8 @@ situation of just wanting to impose a timeout on some code: .. autofunction:: fail_at :with: cancel_scope -Cheat sheet: +Cheat sheet ++++++++++++ * If you want to impose a timeout on a function, but you don't care whether it timed out or not: @@ -597,7 +602,6 @@ which is sometimes useful: .. autofunction:: current_effective_deadline - .. _tasks: Tasks let you do multiple things at once diff --git a/newsfragments/2512.breaking.rst b/newsfragments/2512.breaking.rst new file mode 100644 index 0000000000..f9a6b42631 --- /dev/null +++ b/newsfragments/2512.breaking.rst @@ -0,0 +1,2 @@ +:func:`trio.move_on_after` and :func:`trio.fail_after` previously set the deadline relative to initialization time, instead of more intuitively upon entering the context manager. This might change timeouts if a program relied on this behavior. If you want to restore previous behavior you should instead use ``trio.move_on_at(trio.current_time() + ...)``. +flake8-async has a new rule to catch this, in case you're supporting older trio versions. See :ref:`ASYNC122`. diff --git a/newsfragments/2512.feature.rst b/newsfragments/2512.feature.rst new file mode 100644 index 0000000000..fe9197cde5 --- /dev/null +++ b/newsfragments/2512.feature.rst @@ -0,0 +1 @@ +:meth:`CancelScope.relative_deadline` and :meth:`CancelScope.is_relative` added, as well as a ``relative_deadline`` parameter to ``__init__``. This allows initializing scopes ahead of time, but where the specified relative deadline doesn't count down until the scope is entered. diff --git a/src/trio/_core/_run.py b/src/trio/_core/_run.py index 8921ed7f12..5de75e8e6c 100644 --- a/src/trio/_core/_run.py +++ b/src/trio/_core/_run.py @@ -13,7 +13,7 @@ from contextlib import AbstractAsyncContextManager, contextmanager, suppress from contextvars import copy_context from heapq import heapify, heappop, heappush -from math import inf +from math import inf, isnan from time import perf_counter from typing import ( TYPE_CHECKING, @@ -543,8 +543,21 @@ class CancelScope: cancelled_caught: bool = attrs.field(default=False, init=False) # Constructor arguments: - _deadline: float = attrs.field(default=inf, kw_only=True, alias="deadline") - _shield: bool = attrs.field(default=False, kw_only=True, alias="shield") + _relative_deadline: float = attrs.field(default=inf, kw_only=True) + _deadline: float = attrs.field(default=inf, kw_only=True) + _shield: bool = attrs.field(default=False, kw_only=True) + + def __attrs_post_init__(self) -> None: + if isnan(self._deadline): + raise ValueError("deadline must not be NaN") + if isnan(self._relative_deadline): + raise ValueError("relative deadline must not be NaN") + if self._relative_deadline < 0: + raise ValueError("timeout must be non-negative") + if self._relative_deadline != inf and self._deadline != inf: + raise ValueError( + "Cannot specify both a deadline and a relative deadline", + ) @enable_ki_protection def __enter__(self) -> Self: @@ -554,6 +567,12 @@ def __enter__(self) -> Self: "Each CancelScope may only be used for a single 'with' block", ) self._has_been_entered = True + + if self._relative_deadline != inf: + assert self._deadline == inf + self._deadline = current_time() + self._relative_deadline + self._relative_deadline = inf + if current_time() >= self._deadline: self.cancel() with self._might_change_registered_deadline(): @@ -734,13 +753,70 @@ def deadline(self) -> float: this can be overridden by the ``deadline=`` argument to the :class:`~trio.CancelScope` constructor. """ + if self._relative_deadline != inf: + assert self._deadline == inf + warnings.warn( + DeprecationWarning( + "unentered relative cancel scope does not have an absolute deadline. Use `.relative_deadline`", + ), + stacklevel=2, + ) + return current_time() + self._relative_deadline return self._deadline @deadline.setter def deadline(self, new_deadline: float) -> None: + if isnan(new_deadline): + raise ValueError("deadline must not be NaN") + if self._relative_deadline != inf: + assert self._deadline == inf + warnings.warn( + DeprecationWarning( + "unentered relative cancel scope does not have an absolute deadline. Transforming into an absolute cancel scope. First set `.relative_deadline = math.inf` if you do want an absolute cancel scope.", + ), + stacklevel=2, + ) + self._relative_deadline = inf with self._might_change_registered_deadline(): self._deadline = float(new_deadline) + @property + def relative_deadline(self) -> float: + if self._has_been_entered: + return self._deadline - current_time() + elif self._deadline != inf: + assert self._relative_deadline == inf + raise RuntimeError( + "unentered non-relative cancel scope does not have a relative deadline", + ) + return self._relative_deadline + + @relative_deadline.setter + def relative_deadline(self, new_relative_deadline: float) -> None: + if isnan(new_relative_deadline): + raise ValueError("relative deadline must not be NaN") + if new_relative_deadline < 0: + raise ValueError("relative deadline must be non-negative") + if self._has_been_entered: + with self._might_change_registered_deadline(): + self._deadline = current_time() + float(new_relative_deadline) + elif self._deadline != inf: + assert self._relative_deadline == inf + raise RuntimeError( + "unentered non-relative cancel scope does not have a relative deadline", + ) + else: + self._relative_deadline = new_relative_deadline + + @property + def is_relative(self) -> bool | None: + """Returns None after entering. Returns False if both deadline and + relative_deadline are inf.""" + assert not (self._deadline != inf and self._relative_deadline != inf) + if self._has_been_entered: + return None + return self._relative_deadline != inf + @property def shield(self) -> bool: """Read-write, :class:`bool`, default :data:`False`. So long as diff --git a/src/trio/_core/_tests/test_run.py b/src/trio/_core/_tests/test_run.py index e6ee5e869f..9d2eab787d 100644 --- a/src/trio/_core/_tests/test_run.py +++ b/src/trio/_core/_tests/test_run.py @@ -9,7 +9,7 @@ import types import weakref from contextlib import ExitStack, contextmanager, suppress -from math import inf +from math import inf, nan from typing import TYPE_CHECKING, Any, NoReturn, TypeVar, cast import outcome @@ -365,6 +365,27 @@ async def test_cancel_scope_repr(mock_clock: _core.MockClock) -> None: assert "exited" in repr(scope) +async def test_cancel_scope_validation() -> None: + with pytest.raises( + ValueError, + match="^Cannot specify both a deadline and a relative deadline$", + ): + _core.CancelScope(deadline=7, relative_deadline=3) + scope = _core.CancelScope() + + with pytest.raises(ValueError, match="^deadline must not be NaN$"): + scope.deadline = nan + with pytest.raises(ValueError, match="^relative deadline must not be NaN$"): + scope.relative_deadline = nan + + with pytest.raises(ValueError, match="^relative deadline must be non-negative$"): + scope.relative_deadline = -3 + scope.relative_deadline = 5 + assert scope.relative_deadline == 5 + + # several related tests of CancelScope are implicitly handled by test_timeouts.py + + def test_cancel_points() -> None: async def main1() -> None: with _core.CancelScope() as scope: diff --git a/src/trio/_tests/test_timeouts.py b/src/trio/_tests/test_timeouts.py index b2486c7d2b..515f536c1f 100644 --- a/src/trio/_tests/test_timeouts.py +++ b/src/trio/_tests/test_timeouts.py @@ -4,9 +4,20 @@ import outcome import pytest +import trio + from .. import _core from .._core._tests.tutil import slow -from .._timeouts import * +from .._timeouts import ( + TooSlowError, + fail_after, + fail_at, + move_on_after, + move_on_at, + sleep, + sleep_forever, + sleep_until, +) from ..testing import assert_checkpoints T = TypeVar("T") @@ -155,7 +166,7 @@ async def test_timeouts_raise_value_error() -> None: ): with pytest.raises( ValueError, - match="^(duration|deadline|timeout) must (not )*be (non-negative|NaN)$", + match="^(deadline|`seconds`) must (not )*be (non-negative|NaN)$", ): await fun(val) @@ -169,7 +180,79 @@ async def test_timeouts_raise_value_error() -> None: ): with pytest.raises( ValueError, - match="^(duration|deadline|timeout) must (not )*be (non-negative|NaN)$", + match="^(deadline|`seconds`) must (not )*be (non-negative|NaN)$", ): with cm(val): pass # pragma: no cover + + +async def test_timeout_deadline_on_entry(mock_clock: _core.MockClock) -> None: + rcs = move_on_after(5) + assert rcs.relative_deadline == 5 + + mock_clock.jump(3) + start = _core.current_time() + with rcs as cs: + assert cs.is_relative is None + + # This would previously be start+2 + assert cs.deadline == start + 5 + assert cs.relative_deadline == 5 + + cs.deadline = start + 3 + assert cs.deadline == start + 3 + assert cs.relative_deadline == 3 + + cs.relative_deadline = 4 + assert cs.deadline == start + 4 + assert cs.relative_deadline == 4 + + rcs = move_on_after(5) + assert rcs.shield is False + rcs.shield = True + assert rcs.shield is True + + mock_clock.jump(3) + start = _core.current_time() + with rcs as cs: + assert cs.deadline == start + 5 + + assert rcs is cs + + +async def test_invalid_access_unentered(mock_clock: _core.MockClock) -> None: + cs = move_on_after(5) + mock_clock.jump(3) + start = _core.current_time() + + match_str = "^unentered relative cancel scope does not have an absolute deadline" + with pytest.warns(DeprecationWarning, match=match_str): + assert cs.deadline == start + 5 + mock_clock.jump(1) + # this is hella sketchy, but they *have* been warned + with pytest.warns(DeprecationWarning, match=match_str): + assert cs.deadline == start + 6 + + with pytest.warns(DeprecationWarning, match=match_str): + cs.deadline = 7 + # now transformed into absolute + assert cs.deadline == 7 + assert not cs.is_relative + + cs = move_on_at(5) + + match_str = ( + "^unentered non-relative cancel scope does not have a relative deadline$" + ) + with pytest.raises(RuntimeError, match=match_str): + assert cs.relative_deadline + with pytest.raises(RuntimeError, match=match_str): + cs.relative_deadline = 7 + + +@pytest.mark.xfail(reason="not implemented") +async def test_fail_access_before_entering() -> None: # pragma: no cover + my_fail_at = fail_at(5) + assert my_fail_at.deadline # type: ignore[attr-defined] + my_fail_after = fail_after(5) + assert my_fail_after.relative_deadline # type: ignore[attr-defined] diff --git a/src/trio/_timeouts.py b/src/trio/_timeouts.py index b20a05fb68..1a5e2e1db3 100644 --- a/src/trio/_timeouts.py +++ b/src/trio/_timeouts.py @@ -1,11 +1,15 @@ from __future__ import annotations import math -from contextlib import AbstractContextManager, contextmanager +import sys +from contextlib import contextmanager from typing import TYPE_CHECKING import trio +if TYPE_CHECKING: + from collections.abc import Generator + def move_on_at(deadline: float, *, shield: bool = False) -> trio.CancelScope: """Use as a context manager to create a cancel scope with the given @@ -20,27 +24,38 @@ def move_on_at(deadline: float, *, shield: bool = False) -> trio.CancelScope: ValueError: if deadline is NaN. """ - if math.isnan(deadline): - raise ValueError("deadline must not be NaN") + # CancelScope validates that deadline isn't math.nan return trio.CancelScope(deadline=deadline, shield=shield) -def move_on_after(seconds: float, *, shield: bool = False) -> trio.CancelScope: +def move_on_after( + seconds: float, + *, + shield: bool = False, +) -> trio.CancelScope: """Use as a context manager to create a cancel scope whose deadline is set to now + *seconds*. + The deadline of the cancel scope is calculated upon entering. + Args: seconds (float): The timeout. shield (bool): Initial value for the `~trio.CancelScope.shield` attribute of the newly created cancel scope. Raises: - ValueError: if timeout is less than zero or NaN. + ValueError: if ``seconds`` is less than zero or NaN. """ + # duplicate validation logic to have the correct parameter name if seconds < 0: - raise ValueError("timeout must be non-negative") - return move_on_at(trio.current_time() + seconds, shield=shield) + raise ValueError("`seconds` must be non-negative") + if math.isnan(seconds): + raise ValueError("`seconds` must not be NaN") + return trio.CancelScope( + shield=shield, + relative_deadline=seconds, + ) async def sleep_forever() -> None: @@ -84,7 +99,7 @@ async def sleep(seconds: float) -> None: """ if seconds < 0: - raise ValueError("duration must be non-negative") + raise ValueError("`seconds` must be non-negative") if seconds == 0: await trio.lowlevel.checkpoint() else: @@ -98,9 +113,12 @@ class TooSlowError(Exception): """ -# workaround for PyCharm not being able to infer return type from @contextmanager -# see https://youtrack.jetbrains.com/issue/PY-36444/PyCharm-doesnt-infer-types-when-using-contextlib.contextmanager-decorator -def fail_at(deadline: float, *, shield: bool = False) -> AbstractContextManager[trio.CancelScope]: # type: ignore[misc] +@contextmanager +def fail_at( + deadline: float, + *, + shield: bool = False, +) -> Generator[trio.CancelScope, None, None]: """Creates a cancel scope with the given deadline, and raises an error if it is actually cancelled. @@ -129,15 +147,12 @@ def fail_at(deadline: float, *, shield: bool = False) -> AbstractContextManager[ raise TooSlowError -if not TYPE_CHECKING: - fail_at = contextmanager(fail_at) - - +@contextmanager def fail_after( seconds: float, *, shield: bool = False, -) -> AbstractContextManager[trio.CancelScope]: +) -> Generator[trio.CancelScope, None, None]: """Creates a cancel scope with the given timeout, and raises an error if it is actually cancelled. @@ -148,6 +163,8 @@ def fail_after( it's caught and discarded. When it reaches :func:`fail_after`, then it's caught and :exc:`TooSlowError` is raised in its place. + The deadline of the cancel scope is calculated upon entering. + Args: seconds (float): The timeout. shield (bool): Initial value for the `~trio.CancelScope.shield` attribute @@ -159,6 +176,17 @@ def fail_after( ValueError: if *seconds* is less than zero or NaN. """ - if seconds < 0: - raise ValueError("timeout must be non-negative") - return fail_at(trio.current_time() + seconds, shield=shield) + with move_on_after(seconds, shield=shield) as scope: + yield scope + if scope.cancelled_caught: + raise TooSlowError + + +# Users don't need to know that fail_at & fail_after wraps move_on_at and move_on_after +# and there is no functional difference. So we replace the return value when generating +# documentation. +if "sphinx" in sys.modules: # pragma: no cover + import inspect + + for c in (fail_at, fail_after): + c.__signature__ = inspect.Signature.from_callable(c).replace(return_annotation=trio.CancelScope) # type: ignore[union-attr] diff --git a/test-requirements.txt b/test-requirements.txt index 47aea97d8f..1173aff405 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -105,7 +105,7 @@ pylint==3.2.7 # via -r test-requirements.in pyopenssl==24.2.1 # via -r test-requirements.in -pyright==1.1.378 +pyright==1.1.381 # via -r test-requirements.in pytest==8.3.2 # via -r test-requirements.in