Change fail_after&move_on_after to set deadline relative to entering. Add CancelScope.relative_deadline

newsfragments/2512.deprecated.rst

@@ -0,0 +1,9 @@
+: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. Any code where these two points in time do not line up (according to :func:`trio.current_time`) will now issue a `DeprecationWarning`. To silence the warning and adopt the new behaviour, pass ``timeout_from_enter = True``. If you want to retain current behaviour, you should instead use :func:`trio.move_on_at`/:func:`trio.fail_at`. For example:
+
+.. code-block:: python
+
+    # this is equivalent to how trio.move_on_after(5) would function previously
+    cs = trio.move_on_at(trio.current_time() + 5)
+    ...
+    with cs:
+        ...

newsfragments/2512.feature.rst

@@ -0,0 +1,13 @@
+:func:`trio.CancelScope` now has an attribute ``relative_deadline`` that can be used together with, or instead of, ``deadline``. :func:`trio.move_on_after` and :func:`trio.fail_after` now use this functionality in order to resolve the absolute deadline upon entering the context manager.
+
+If setting both ``deadline`` and ``relative_deadline`` before entering the cm, the deadline will be set to ``min(deadline, trio.current_time() + relative_deadline)``.
+Accessing ``relative_deadline`` after entering will return remaining time until deadline (i.e. ``deadline - trio.current_time()``. Setting ``relative_deadline`` after entering will set ``deadline`` to ``trio.current_time() + relative_deadline``
+
+Example:
+
+.. code-block:: python
+
+   my_cs = trio.CancelScope(relative_deadline = 5)
+   ...
+   with my_cs:  # my_cs.deadline will now be 5 seconds into the future
+       ...

src/trio/_core/_run.py

@@ -543,16 +543,60 @@ class CancelScope:
 
     # Constructor arguments:
     _deadline: float = attrs.field(default=inf, kw_only=True, alias="deadline")
+    # use float|None, or math.inf?
+    # or take it as an opportunity to also introduce deadline=None?
+    _relative_deadline: float | None = attrs.field(
+        default=None, kw_only=True, alias="relative_deadline"
+    )
     _shield: bool = attrs.field(default=False, kw_only=True, alias="shield")
 
+    # introduced during deprecation period of handling relative timeouts from
+    # initialization
+    _timeout_from_enter: bool = attrs.field(
+        default=True, kw_only=True, alias="_timeout_from_enter"
+    )
+    _creation_time: float | None = attrs.field(default=None, alias="_creation_time")
+
     @enable_ki_protection
     def __enter__(self) -> Self:
         task = _core.current_task()
         if self._has_been_entered:
             raise RuntimeError(
                 "Each CancelScope may only be used for a single 'with' block"
             )
+
+        if (
+            self._creation_time is not None
+            and abs(self._creation_time - current_time()) > 0.01
+            and not self._timeout_from_enter
+        ):
+            # not using warn_deprecated because the message template is a weird fit
+            # TODO: mention versions in the message?
+            warnings.warn(
+                DeprecationWarning(
+                    "`move_on_after` and `fail_after` will change behaviour to "
+                    "start the deadline relative to entering the cm, instead of "
+                    "at creation time. To silence this warning and opt into the "
+                    "new behaviour, pass `timeout_from_enter=True`. "
+                    "To keep old behaviour, use `move_on_at(trio.current_time() + x)` "
+                    "(or `fail_at`), where `x` is the previous timeout length. "
+                    "See https://github.com/python-trio/trio/issues/2512"
+                ),
+                stacklevel=2,
+            )
+            assert (
+                self._relative_deadline is not None
+            ), "User is manually passing `_timeout_from_enter=False`. Don't do that."
+            self._relative_deadline -= current_time() - self._creation_time
+
         self._has_been_entered = True
+
+        if (
+            self._relative_deadline is not None
+            and self._relative_deadline + current_time() < self._deadline
+        ):
+            self._deadline = self._relative_deadline + current_time()
+
         if current_time() >= self._deadline:
             self.cancel()
         with self._might_change_registered_deadline():
@@ -740,6 +784,25 @@ def deadline(self, new_deadline: float) -> None:
         with self._might_change_registered_deadline():
             self._deadline = float(new_deadline)
 
+    @property
+    def relative_deadline(self) -> float | None:
+        """TODO: write docstring"""
+        if self._has_been_entered:
+            if self.deadline == inf:
+                return None
+            return self._deadline - current_time()
+        return self._relative_deadline
+
+    @relative_deadline.setter
+    def relative_deadline(self, relative_deadline: float | None) -> None:
+        self._relative_deadline = relative_deadline
+        if self._has_been_entered:
+            with self._might_change_registered_deadline():
+                if relative_deadline is None:
+                    self._deadline = inf
+                else:
+                    self._deadline = current_time() + relative_deadline
+
     @property
     def shield(self) -> bool:
         """Read-write, :class:`bool`, default :data:`False`. So long as

src/trio/_core/_tests/test_run.py

@@ -601,6 +601,60 @@ async def test_basic_timeout(mock_clock: _core.MockClock) -> None:
             await _core.checkpoint()
 
 
+async def test_relative_timeout(mock_clock: _core.MockClock) -> None:
+    # test defaults
+    scope = _core.CancelScope()
+    assert scope.deadline == inf
+    assert scope.relative_deadline is None
+
+    # setting relative_deadline before entering does not modify deadline
+    scope.relative_deadline = 1
+    assert scope.deadline == inf
+    assert scope.relative_deadline == 1
+
+    # check that setting either deadline updates the other (after entering)
+    start = _core.current_time()
+    with scope:
+        assert scope.deadline == start + 1
+        scope.relative_deadline = 2
+        assert scope.deadline == start + 2
+        scope.deadline = start + 3
+        assert scope.relative_deadline == 3
+
+    # relative_deadline < deadline, deadline is updated
+    with _core.CancelScope(relative_deadline=1, deadline=start + 2) as scope:
+        assert scope.deadline == start + 1
+        assert scope.relative_deadline == 1
+
+    # deadline < relative_deadline, relative_deadline is updated
+    with _core.CancelScope(relative_deadline=2, deadline=start + 1):
+        assert scope.deadline == start + 1
+        assert scope.relative_deadline == 1
+
+    # once entered, accessing relative_deadline returns deadline-now()
+    # and setting it calculates it relative to now()
+    start = _core.current_time()
+    with _core.CancelScope(relative_deadline=3) as scope:
+        mock_clock.jump(1)
+        assert scope.relative_deadline == 2
+        mock_clock.jump(1)
+        assert scope.relative_deadline == 1
+        scope.relative_deadline = 3
+        mock_clock.jump(1)
+        assert scope.relative_deadline == 2
+
+    # no deadline / infinite deadline
+    with _core.CancelScope() as scope:
+        assert scope.deadline == inf
+        assert scope.relative_deadline is None
+
+    start = _core.current_time()
+    with _core.CancelScope(relative_deadline=1) as scope:
+        assert scope.deadline == start + 1
+        scope.relative_deadline = None
+        assert scope.deadline == inf
+
+
 async def test_cancel_scope_nesting() -> None:
     # Nested scopes: if two triggering at once, the outer one wins
     with _core.CancelScope() as scope1:

src/trio/_tests/test_timeouts.py

@@ -1,12 +1,20 @@
 import time
-from typing import Awaitable, Callable, TypeVar
+from typing import Awaitable, Callable, ContextManager, TypeVar
 
 import outcome
 import pytest
 
 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_until,
+)
 from ..testing import assert_checkpoints
 
 T = TypeVar("T")
@@ -115,7 +123,10 @@ async def test_timeouts_raise_value_error() -> None:
         ):
             await fun(val)
 
-    for cm, val in (
+    cm: Callable[[float], ContextManager[_core.CancelScope]]
+    # mypy resolves the tuple as containing `Callable[[float], object]`, failing to see
+    # that both callables are compatible with returning `Contextmanager[CancelScope]`
+    for cm, val in (  # type: ignore[assignment]
         (fail_after, -1),
         (fail_after, nan),
         (fail_at, nan),
@@ -129,3 +140,40 @@ async def test_timeouts_raise_value_error() -> None:
         ):
             with cm(val):
                 pass  # pragma: no cover
+
+
+async def test_timeout_deadline_on_entry(mock_clock: _core.MockClock) -> None:
+    cs = move_on_after(5, timeout_from_enter=True)
+    assert cs.relative_deadline == 5
+    mock_clock.jump(3)
+    start = _core.current_time()
+    with cs:
+        # This would previously be start+2
+        assert cs.deadline == start + 5
+        assert cs.relative_deadline == 5
+
+    # because fail_after is a wrapping contextmanager we cannot directly inspect
+    # cs_gen.relative_deadline before entering
+    cs_gen = fail_after(5, timeout_from_enter=True)
+    mock_clock.jump(3)
+    start = _core.current_time()
+    with cs_gen as cs:
+        assert cs.deadline == start + 5
+        assert cs.relative_deadline == 5
+
+
+async def test_timeout_deadline_not_on_entry(mock_clock: _core.MockClock) -> None:
+    """Test that not setting timeout_from_enter gives a DeprecationWarning and
+    retains old behaviour."""
+    with pytest.warns(DeprecationWarning, match="issues/2512"):
+        cs = move_on_after(5)
+        mock_clock.jump(3)
+        with cs:
+            assert cs.deadline - _core.current_time() == 2
+            assert cs.relative_deadline == 2
+    with pytest.warns(DeprecationWarning, match="issues/2512"):
+        cs_gen = fail_after(5)
+        mock_clock.jump(3)
+        with cs_gen as cs:
+            assert cs.deadline - _core.current_time() == 2
+            assert cs.relative_deadline == 2

src/trio/_timeouts.py

@@ -1,11 +1,17 @@
 from __future__ import annotations
 
 import math
-from contextlib import AbstractContextManager, contextmanager
+from contextlib import contextmanager
 from typing import TYPE_CHECKING
 
 import trio
 
+from ._util import final
+
+if TYPE_CHECKING:
+    from collections.abc import Generator
+    from types import TracebackType
+
 
 def move_on_at(deadline: float) -> trio.CancelScope:
     """Use as a context manager to create a cancel scope with the given
@@ -23,9 +29,14 @@ def move_on_at(deadline: float) -> trio.CancelScope:
     return trio.CancelScope(deadline=deadline)
 
 
-def move_on_after(seconds: float) -> trio.CancelScope:
+def move_on_after(
+    seconds: float,
+    *,
+    timeout_from_enter: bool = False,
+    _creation_time: float | None = None,
+) -> trio.CancelScope:
     """Use as a context manager to create a cancel scope whose deadline is
-    set to now + *seconds*.
+    set to creation time + *seconds*.
 
     Args:
       seconds (float): The timeout.
@@ -36,7 +47,15 @@ def move_on_after(seconds: float) -> trio.CancelScope:
     """
     if seconds < 0:
         raise ValueError("timeout must be non-negative")
-    return move_on_at(trio.current_time() + seconds)
+    if math.isnan(seconds):
+        raise ValueError("timeout must not be NaN")
+    if _creation_time is None:
+        _creation_time = trio.current_time()
+    return trio.CancelScope(
+        relative_deadline=seconds,
+        _timeout_from_enter=timeout_from_enter,
+        _creation_time=_creation_time,
+    )
 
 
 async def sleep_forever() -> None:
@@ -94,9 +113,8 @@ 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) -> AbstractContextManager[trio.CancelScope]:  # type: ignore[misc]
+@contextmanager
+def fail_at(deadline: float) -> Generator[trio.CancelScope, None, None]:
     """Creates a cancel scope with the given deadline, and raises an error if it
     is actually cancelled.
 
@@ -123,11 +141,14 @@ def fail_at(deadline: float) -> AbstractContextManager[trio.CancelScope]:  # typ
         raise TooSlowError
 
 
-if not TYPE_CHECKING:
-    fail_at = contextmanager(fail_at)
+# This was previously a simple @contextmanager-based cm, but in order to save
+# the current time at initialization it needed to be class-based.
+# Once that change of behaviour has gone through and the old functionality is removed
+# it can be reverted.
 
 
-def fail_after(seconds: float) -> AbstractContextManager[trio.CancelScope]:
+@final
+class fail_after:
     """Creates a cancel scope with the given timeout, and raises an error if
     it is actually cancelled.
 
@@ -138,6 +159,9 @@ def fail_after(seconds: float) -> AbstractContextManager[trio.CancelScope]:
     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 at creation time, not upon
+    entering the context manager.
+
     Args:
       seconds (float): The timeout.
 
@@ -147,6 +171,30 @@ def fail_after(seconds: float) -> AbstractContextManager[trio.CancelScope]:
       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)
+
+    def __init__(self, seconds: float, *, timeout_from_enter: bool = False):
+        self._seconds = seconds
+        self._timeout_from_enter = timeout_from_enter
+        self._creation_time = trio.current_time()
+        self._scope: trio.CancelScope | None = None
+
+    def __enter__(self) -> trio.CancelScope:
+        self._scope = trio.move_on_after(
+            self._seconds,
+            timeout_from_enter=self._timeout_from_enter,
+            _creation_time=self._creation_time,
+        )
+        return self._scope.__enter__()
+
+    def __exit__(
+        self,
+        exc_type: type[BaseException] | None,
+        exc_value: BaseException | None,
+        traceback: TracebackType | None,
+    ) -> bool | None:
+        if self._scope is None:  # pragma: no cover
+            raise RuntimeError("__exit__ called before __enter__")
+        result = self._scope.__exit__(exc_type, exc_value, traceback)
+        if self._scope.cancelled_caught:
+            raise TooSlowError
+        return result