From 80b51feeff494823813c2d7b917fd9d6c5f25012 Mon Sep 17 00:00:00 2001 From: Joshua Oreman Date: Tue, 1 Jan 2019 20:46:37 -0800 Subject: [PATCH 01/12] Add support for unbound and linked cancel scopes --- docs/source/history.rst | 2 +- docs/source/reference-core.rst | 74 +++++-- newsfragments/607.feature.rst | 6 + newsfragments/607.removal.rst | 3 + notes-to-self/graceful-shutdown-idea.py | 19 +- trio/__init__.py | 4 +- trio/_core/_exceptions.py | 2 +- trio/_core/_run.py | 258 +++++++++++++++++------- trio/_core/_traps.py | 2 +- trio/_core/tests/test_ki.py | 2 +- trio/_core/tests/test_parking_lot.py | 2 +- trio/_core/tests/test_run.py | 249 +++++++++++++++++++---- trio/_file_io.py | 2 +- trio/_highlevel_generic.py | 2 +- trio/_subprocess.py | 4 +- trio/_sync.py | 2 +- trio/_timeouts.py | 2 +- trio/testing/_check_streams.py | 6 +- trio/testing/_mock_clock.py | 2 +- trio/tests/test_file_io.py | 2 +- trio/tests/test_socket.py | 8 +- trio/tests/test_sync.py | 2 +- trio/tests/test_testing.py | 2 +- trio/tests/test_threads.py | 6 +- trio/tests/test_util.py | 2 +- 25 files changed, 496 insertions(+), 169 deletions(-) create mode 100644 newsfragments/607.feature.rst create mode 100644 newsfragments/607.removal.rst diff --git a/docs/source/history.rst b/docs/source/history.rst index 945f7b7848..345b63415e 100644 --- a/docs/source/history.rst +++ b/docs/source/history.rst @@ -212,7 +212,7 @@ Deprecations and Removals ~~~~~~~~~~~~~~~~~~~~~~~~~ - Attempting to explicitly raise :exc:`trio.Cancelled` will cause a :exc:`RuntimeError`. - :meth:`cancel_scope.cancel() ` should + :meth:`cancel_scope.cancel() ` should be used instead. (`#342 `__) diff --git a/docs/source/reference-core.rst b/docs/source/reference-core.rst index 50f00c18fc..7c7d6592e2 100644 --- a/docs/source/reference-core.rst +++ b/docs/source/reference-core.rst @@ -380,7 +380,7 @@ whether this scope caught a :exc:`Cancelled` exception:: The ``cancel_scope`` object also allows you to check or adjust this scope's deadline, explicitly trigger a cancellation without waiting for the deadline, check if the scope has already been cancelled, and -so forth – see :func:`open_cancel_scope` below for the full details. +so forth – see :class:`CancelScope` below for the full details. .. _blocking-cleanup-example: @@ -415,7 +415,7 @@ Of course, if you really want to make another blocking call in your cleanup handler, trio will let you; it's trying to prevent you from accidentally shooting yourself in the foot. Intentional foot-shooting is no problem (or at least – it's not trio's problem). To do this, -create a new scope, and set its :attr:`~The cancel scope interface.shield` +create a new scope, and set its :attr:`~CancelScope.shield` attribute to :data:`True`:: with trio.move_on_after(TIMEOUT): @@ -494,14 +494,11 @@ but *will* still close the underlying socket before raising Cancellation API details ~~~~~~~~~~~~~~~~~~~~~~~~ -The primitive operation for creating a new cancellation scope is: +:func:`move_on_after` and all the other cancellation facilities provided +by Trio are ultimately implemented in terms of :class:`CancelScope` +objects. -.. autofunction:: open_cancel_scope - :with: cancel_scope - -Cancel scope objects provide the following interface: - -.. interface:: The cancel scope interface +.. autoclass:: trio.CancelScope .. attribute:: deadline @@ -525,7 +522,7 @@ Cancel scope objects provide the following interface: Defaults to :data:`math.inf`, which means "no deadline", though this can be overridden by the ``deadline=`` argument to - :func:`~trio.open_cancel_scope`. + the :class:`~trio.CancelScope` constructor. .. attribute:: shield @@ -536,7 +533,7 @@ Cancel scope objects provide the following interface: :exc:`~trio.Cancelled` exceptions from (1) this scope, or (2) scopes inside this scope. You can modify this attribute:: - with trio.open_cancel_scope() as cancel_scope: + with trio.CancelScope() as cancel_scope: cancel_scope.shield = True # This cannot be interrupted by any means short of # killing the process: @@ -547,7 +544,7 @@ Cancel scope objects provide the following interface: await sleep(10) Defaults to :data:`False`, though this can be overridden by the - ``shield=`` argument to :func:`~trio.open_cancel_scope`. + ``shield=`` argument to the :class:`~trio.CancelScope` constructor. .. method:: cancel() @@ -564,24 +561,65 @@ Cancel scope objects provide the following interface: exception, and (2) this scope is the one that was responsible for triggering this :exc:`~trio.Cancelled` exception. + If the same :class:`CancelScope` is reused for multiple ``with`` + blocks, the :attr:`cancelled_caught` attribute applies to the + most recent ``with`` block. (It is reset to :data:`False` each + time a new ``with`` block is entered.) + .. attribute:: cancel_called Readonly :class:`bool`. Records whether cancellation has been requested for this scope, either by an explicit call to :meth:`cancel` or by the deadline expiring. - This attribute being True does *not* necessarily mean that - the code within the scope has been, or will be, affected by - the cancellation. For example, if :meth:`cancel` was called - just before the scope exits, when it's too late to deliver - a :exc:`~trio.Cancelled` exception, then this attribute will - still be True. + This attribute being True does *not* necessarily mean that the + code within the scope has been, or will be, affected by the + cancellation. For example, if :meth:`cancel` was called after + the last checkpoint in the ``with`` block, when it's too late to + deliver a :exc:`~trio.Cancelled` exception, then this attribute + will still be True. This attribute is mostly useful for debugging and introspection. If you want to know whether or not a chunk of code was actually cancelled, then :attr:`cancelled_caught` is usually more appropriate. + .. method:: open_branch(*, deadline=math.inf, shield=None) + + Return another :class:`CancelScope` object that automatically + becomes cancelled when this one does. We say that the returned + cancel scope is a "branch" of this "source" cancel scope. + + The relationship between the source cancel scope and the new + branch is one-way: if the source cancel scope becomes cancelled, + all of its branches do too, but any of the branches can + independently become cancelled without affecting the + source. Each branch has its own :attr:`deadline`, which is + :data:`math.inf` (not inherited from the source scope) if the + ``deadline`` argument is unspecified; the expiry of a branch's + deadline cancels that branch only. + + The new branch inherits its initial :attr:`shield` attribute + from the source, unless overridden via the ``shield`` argument. + The branch :attr:`shield` may be changed without affecting the + source, but changes to the source :attr:`shield` will be + propagated to all branches, overriding any local :attr:`shield` + value they've previously set. + + Multiple layers of cancel scope linkage are supported. The + cancellation of any source scope affects all its branches, all + their branches, and so on. + + .. attribute:: branches + + An iterable yielding all the other cancel scopes that will + become cancelled when this one does. That includes all the + branches of this cancel scope and all of their branches, recursively. + Cancel scopes track their branches by weak reference, so a scope + may no longer be reflected in :attr:`branches` once it has no other + references active. + + Trio also provides several convenience functions for the common situation of just wanting to impose a timeout on some code: diff --git a/newsfragments/607.feature.rst b/newsfragments/607.feature.rst new file mode 100644 index 0000000000..e29d4ddd83 --- /dev/null +++ b/newsfragments/607.feature.rst @@ -0,0 +1,6 @@ +Add support for "unbound cancel scopes": you can now construct a +:class:`trio.CancelScope` without entering its context, e.g. so you +can pass it to another task which will use it to wrap some work that +you want to be able to cancel from afar. Add the ability to create a +derived :class:`~trio.CancelScope` that will become cancelled when its +parent does, using :meth:`~trio.CancelScope.open_branch`. diff --git a/newsfragments/607.removal.rst b/newsfragments/607.removal.rst new file mode 100644 index 0000000000..1500b812ad --- /dev/null +++ b/newsfragments/607.removal.rst @@ -0,0 +1,3 @@ +Deprecate ``trio.open_cancel_scope`` in favor of :class:`trio.CancelScope`, +which more clearly reflects that creating a cancel scope is just an ordinary +object construction and does not need to be immediately paired with entering it. diff --git a/notes-to-self/graceful-shutdown-idea.py b/notes-to-self/graceful-shutdown-idea.py index 28164cb495..fa0c06d6b1 100644 --- a/notes-to-self/graceful-shutdown-idea.py +++ b/notes-to-self/graceful-shutdown-idea.py @@ -3,28 +3,17 @@ class GracefulShutdownManager: def __init__(self): - self._shutting_down = False - self._cancel_scopes = set() + self._root_scope = trio.CancelScope() def start_shutdown(self): - self._shutting_down = True - for cancel_scope in self._cancel_scopes: - cancel_scope.cancel() + self._root_scope.cancel() - @contextmanager def cancel_on_graceful_shutdown(self): - with trio.open_cancel_scope() as cancel_scope: - self._cancel_scopes.add(cancel_scope) - if self._shutting_down: - cancel_scope.cancel() - try: - yield - finally: - self._cancel_scopes.remove(cancel_scope) + return self._root_scope.open_branch() @property def shutting_down(self): - return self._shutting_down + return self._root_scope.cancel_called # Code can check gsm.shutting_down occasionally at appropriate points to see # if it should exit. diff --git a/trio/__init__.py b/trio/__init__.py index 2cab1e5818..e78ad343b9 100644 --- a/trio/__init__.py +++ b/trio/__init__.py @@ -18,8 +18,8 @@ from ._core import ( TrioInternalError, RunFinishedError, WouldBlock, Cancelled, BusyResourceError, ClosedResourceError, MultiError, run, open_nursery, - open_cancel_scope, current_effective_deadline, TASK_STATUS_IGNORED, - current_time, BrokenResourceError, EndOfChannel + CancelScope, open_cancel_scope, current_effective_deadline, + TASK_STATUS_IGNORED, current_time, BrokenResourceError, EndOfChannel ) from ._timeouts import ( diff --git a/trio/_core/_exceptions.py b/trio/_core/_exceptions.py index fd20406ae0..937da2328e 100644 --- a/trio/_core/_exceptions.py +++ b/trio/_core/_exceptions.py @@ -49,7 +49,7 @@ class Cancelled(BaseException): Attempting to raise :exc:`Cancelled` yourself will cause a :exc:`RuntimeError`. It would not be associated with a cancel scope and thus not be caught by Trio. Use - :meth:`cancel_scope.cancel() ` + :meth:`cancel_scope.cancel() ` instead. .. note:: diff --git a/trio/_core/_run.py b/trio/_core/_run.py index 36112784c4..7cd4828cb7 100644 --- a/trio/_core/_run.py +++ b/trio/_core/_run.py @@ -5,6 +5,7 @@ import select import sys import threading +import weakref from collections import deque import collections.abc from contextlib import contextmanager, closing @@ -35,15 +36,16 @@ WaitTaskRescheduled, ) from .. import _core +from .._deprecate import deprecated # At the bottom of this file there's also some "clever" code that generates # wrapper functions for runner and io manager methods, and adds them to # __all__. These are all re-exported as part of the 'trio' or 'trio.hazmat' # namespaces. __all__ = [ - "Task", "run", "open_nursery", "open_cancel_scope", "checkpoint", - "current_task", "current_effective_deadline", "checkpoint_if_cancelled", - "TASK_STATUS_IGNORED" + "Task", "run", "open_nursery", "open_cancel_scope", "CancelScope", + "checkpoint", "current_task", "current_effective_deadline", + "checkpoint_if_cancelled", "TASK_STATUS_IGNORED" ] GLOBAL_RUN_CONTEXT = threading.local() @@ -116,28 +118,163 @@ def deadline_to_sleep_time(self, deadline): ################################################################ -@attr.s(cmp=False, hash=False, repr=False) +@attr.s(cmp=False, repr=False) class CancelScope: - _tasks = attr.ib(default=attr.Factory(set)) - _scope_task = attr.ib(default=None) - _effective_deadline = attr.ib(default=inf) - _deadline = attr.ib(default=inf) - _shield = attr.ib(default=False) - cancel_called = attr.ib(default=False) - cancelled_caught = attr.ib(default=False) - - @staticmethod - def _create(deadline, shield): + """A *cancellation scope*: the link between a unit of cancellable + work and Trio's cancellation system. + + A :class:`CancelScope` becomes associated with some cancellable work + when it is used as a context manager surrounding that work:: + + cancel_scope = trio.CancelScope() + ... + with cancel_scope: + await long_running_operation() + + Inside the ``with`` block, a cancellation of ``cancel_scope`` (via + a call to its :meth:`cancel` method or via the expiry of its + :attr:`deadline`) will immediately interrupt the + ``long_running_operation()`` by raising :exc:`Cancelled` at its + next :ref:`checkpoint `. + + The context manager ``__enter__`` returns the :class:`CancelScope` + object itself, so you can also write ``with trio.CancelScope() as + cancel_scope:``. + + If a cancel scope becomes cancelled before entering its ``with`` block, + the :exc:`Cancelled` exception will be raised at the first + checkpoint inside the ``with`` block. This allows a + :class:`CancelScope` to be created in one :ref:`task ` and + passed to another, so that the first task can later cancel some work + inside the second. + + Cancel scopes are reusable: once you exit the ``with`` block, you + can use the same :class:`CancelScope` object to wrap another chunk + of work. (The cancellation state doesn't change; once a cancel + scope becomes cancelled, it stays cancelled.) This can be useful + if you want a cancellation to be able to interrupt some operations + in a loop but not others:: + + cancel_scope = trio.CancelScope(deadline=...) + while True: + with cancel_scope: + request = await get_next_request() + response = await handle_request(request) + await send_response(response) + + Cancel scopes are *not* reentrant: you can't enter a second + ``with`` block using the same :class:`CancelScope` while the first + one is still active. (You'll get a :exc:`RuntimeError` if you + try.) If you want multiple blocks of work to be cancelled with + the same call to :meth:`cancel` or at the expiry of the same + deadline, see the :meth:`open_branch` method. + + The :class:`CancelScope` constructor takes initial values for the + cancel scope's :attr:`deadline` and :attr:`shield` attributes; these + may be freely modified after construction, whether or not the scope + has been entered yet, and changes take immediate effect. + + """ + + _tasks = attr.ib(factory=set, init=False) + _scope_task = attr.ib(default=None, init=False) + _effective_deadline = attr.ib(default=inf, init=False) + _branches = attr.ib(default=(), init=False) + cancel_called = attr.ib(default=False, init=False) + cancelled_caught = attr.ib(default=False, init=False) + _deadline = attr.ib(default=inf, kw_only=True) + _shield = attr.ib(default=False, kw_only=True) + + @enable_ki_protection + def __enter__(self): task = _core.current_task() - scope = CancelScope() - scope._scope_task = task - scope._add_task(task) - scope.deadline = deadline - scope.shield = shield - return scope + if self._scope_task is not None: + raise RuntimeError( + "cancel scope may not be entered while it is already " + "active{}; try `with cancel_scope.branch() as new_scope:` " + "instead".format( + "" if self._scope_task is task else + " in another task ({!r})".format(self._scope_task.name) + ) + ) + self._scope_task = task + self.cancelled_caught = False + with self._might_change_effective_deadline(): + self._add_task(task) + return self + + @enable_ki_protection + def __exit__(self, etype, exc, tb): + # NB: NurseryManager calls _close() directly rather than __exit__(), + # so __exit__() must be just _close() plus this logic for adapting + # the exception-filtering result to the context manager API. + + # Tracebacks show the 'raise' line below out of context, so let's give + # this variable a name that makes sense out of context. + remaining_error_after_cancel_scope = self._close(exc) + if remaining_error_after_cancel_scope is None: + return True + elif remaining_error_after_cancel_scope is exc: + return False + else: + # Copied verbatim from MultiErrorCatcher. Python doesn't + # allow us to encapsulate this __context__ fixup. + old_context = remaining_error_after_cancel_scope.__context__ + try: + raise remaining_error_after_cancel_scope + finally: + _, value, _ = sys.exc_info() + assert value is remaining_error_after_cancel_scope + value.__context__ = old_context + + def open_branch(self, *, deadline=inf, shield=None): + if not hasattr(self._branches, "add"): + # We keep _branches as an empty tuple until the first branch + # is created, to avoid creating a fairly heavyweight + # WeakSet on every cancel scope object + self._branches = weakref.WeakSet() + + if shield is None: + shield = self._shield + child = CancelScope(deadline=deadline, shield=shield) + if self.cancel_called: + child.cancel() + self._branches.add(child) + return child + + @property + def branches(self): + for branch in self._branches: + yield branch + yield from branch.branches def __repr__(self): - return "".format(id(self)) + if self._scope_task is None: + binding = "unbound" + else: + binding = "bound to {!r}".format(self._scope_task.name) + if len(self._tasks) > 1: + binding += " and its {} children".format(len(self._tasks) - 1) + + if self.cancel_called: + state = ", cancelled" + elif self.deadline == inf: + state = "" + else: + now = current_time() + if now >= self.deadline: + state = ", cancel soon" + else: + state = ", cancel in {:.2f}sec".format(self.deadline - now) + + if self._branches: + branches = ", {} branches".format(sum(1 for _ in self.branches)) + else: + branches = "" + + return "".format( + id(self), binding, state, branches + ) @contextmanager @enable_ki_protection @@ -172,13 +309,24 @@ def shield(self): return self._shield @shield.setter + @enable_ki_protection def shield(self, new_value): if not isinstance(new_value, bool): raise TypeError("shield must be a bool") + + # Set shielding for us and all branches, then check cancellations + # for us and all branches if shielding was turned off. This ordering + # ensures that each task receives the cancellation for its + # outermost cancelled scope. self._shield = new_value + for branch in self.branches: + branch._shield = new_value if not self._shield: for task in self._tasks: task._attempt_delivery_of_any_pending_cancel() + for branch in self.branches: + for task in branch._tasks: + task._attempt_delivery_of_any_pending_cancel() def _cancel_no_notify(self): # returns the affected tasks @@ -191,7 +339,16 @@ def _cancel_no_notify(self): @enable_ki_protection def cancel(self): - for task in self._cancel_no_notify(): + # Set cancel_called and collect affected tasks for us and all + # branches, then check cancellations for each affected task. + # This ordering ensures that each task receives the + # cancellation for its outermost cancelled scope. + affected_tasks = self._cancel_no_notify() + if self._branches: + affected_tasks = set(affected_tasks) + for branch in self.branches: + affected_tasks.update(branch._cancel_no_notify()) + for task in affected_tasks: task._attempt_delivery_of_any_pending_cancel() def _add_task(self, task): @@ -199,8 +356,7 @@ def _add_task(self, task): task._cancel_stack.append(self) def _remove_task(self, task): - with self._might_change_effective_deadline(): - self._tasks.remove(task) + self._tasks.remove(task) assert task._cancel_stack[-1] is self task._cancel_stack.pop() @@ -225,52 +381,18 @@ def _exc_filter(self, exc): return exc def _close(self, exc): - self._remove_task(self._scope_task) + with self._might_change_effective_deadline(): + self._remove_task(self._scope_task) + self._scope_task = None if exc is not None: - filtered_exc = MultiError.filter(self._exc_filter, exc) - return filtered_exc - - -# We explicitly avoid @contextmanager since it adds extraneous stack frames -# to exceptions. -@attr.s -class CancelScopeManager: - - _deadline = attr.ib(default=inf) - _shield = attr.ib(default=False) - - @enable_ki_protection - def __enter__(self): - self._scope = CancelScope._create(self._deadline, self._shield) - return self._scope - - @enable_ki_protection - def __exit__(self, etype, exc, tb): - # Tracebacks show the 'raise' line below out of context, so let's give - # this variable a name that makes sense out of context. - remaining_error_after_cancel_scope = self._scope._close(exc) - if remaining_error_after_cancel_scope is None: - return True - elif remaining_error_after_cancel_scope is exc: - return False - else: - # Copied verbatim from MultiErrorCatcher. Python doesn't - # allow us to encapsulate this __context__ fixup. - old_context = remaining_error_after_cancel_scope.__context__ - try: - raise remaining_error_after_cancel_scope - finally: - _, value, _ = sys.exc_info() - assert value is remaining_error_after_cancel_scope - value.__context__ = old_context + return MultiError.filter(self._exc_filter, exc) + return None +@deprecated("0.10.0", issue=607, instead="trio.CancelScope") def open_cancel_scope(*, deadline=inf, shield=False): - """Returns a context manager which creates a new cancellation scope. - - """ - - return CancelScopeManager(deadline, shield) + """Returns a context manager which creates a new cancellation scope.""" + return CancelScope(deadline=deadline, shield=shield) ################################################################ @@ -376,7 +498,7 @@ class NurseryManager: @enable_ki_protection async def __aenter__(self): - self._scope = CancelScope._create(deadline=inf, shield=False) + self._scope = CancelScope().__enter__() self._nursery = Nursery(current_task(), self._scope) return self._nursery @@ -1587,7 +1709,7 @@ async def checkpoint(): :func:`checkpoint`.) """ - with open_cancel_scope(deadline=-inf): + with CancelScope(deadline=-inf): await _core.wait_task_rescheduled(lambda _: _core.Abort.SUCCEEDED) diff --git a/trio/_core/_traps.py b/trio/_core/_traps.py index c51cd252ea..f7cf934489 100644 --- a/trio/_core/_traps.py +++ b/trio/_core/_traps.py @@ -37,7 +37,7 @@ async def cancel_shielded_checkpoint(): Equivalent to (but potentially more efficient than):: - with trio.open_cancel_scope(shield=True): + with trio.CancelScope(shield=True): await trio.hazmat.checkpoint() """ diff --git a/trio/_core/tests/test_ki.py b/trio/_core/tests/test_ki.py index 6c95325735..e0375e669c 100644 --- a/trio/_core/tests/test_ki.py +++ b/trio/_core/tests/test_ki.py @@ -410,7 +410,7 @@ async def main(): @_core.enable_ki_protection async def main(): assert _core.currently_ki_protected() - with _core.open_cancel_scope() as cancel_scope: + with _core.CancelScope() as cancel_scope: cancel_scope.cancel() with pytest.raises(_core.Cancelled): await _core.checkpoint() diff --git a/trio/_core/tests/test_parking_lot.py b/trio/_core/tests/test_parking_lot.py index de6de63081..95e4a96b50 100644 --- a/trio/_core/tests/test_parking_lot.py +++ b/trio/_core/tests/test_parking_lot.py @@ -82,7 +82,7 @@ async def waiter(i, lot): async def cancellable_waiter(name, lot, scopes, record): - with _core.open_cancel_scope() as scope: + with _core.CancelScope() as scope: scopes[name] = scope record.append("sleep {}".format(name)) try: diff --git a/trio/_core/tests/test_run.py b/trio/_core/tests/test_run.py index c959fff558..afaf9e447c 100644 --- a/trio/_core/tests/test_run.py +++ b/trio/_core/tests/test_run.py @@ -1,6 +1,7 @@ import contextvars import functools import platform +import re import sys import threading import time @@ -17,7 +18,7 @@ from .tutil import check_sequence_matches, gc_collect_harder from ... import _core -from ..._timeouts import sleep +from ..._timeouts import sleep, fail_after from ..._util import aiter_compat from ...testing import ( wait_all_tasks_blocked, @@ -336,7 +337,7 @@ async def child(): await _core.checkpoint() await _core.checkpoint() - with _core.open_cancel_scope(deadline=_core.current_time() + 5): + with _core.CancelScope(deadline=_core.current_time() + 5): stats = _core.current_statistics() print(stats) assert stats.seconds_to_next_deadline == 5 @@ -540,15 +541,28 @@ async def main(): assert "Instrument has been disabled" in caplog.records[0].message -async def test_cancel_scope_repr(): - # Trivial smoke test - with _core.open_cancel_scope() as scope: - assert repr(scope).startswith(" Date: Tue, 1 Jan 2019 21:06:25 -0800 Subject: [PATCH 02/12] yapf nit --- trio/_core/tests/test_run.py | 1 - 1 file changed, 1 deletion(-) diff --git a/trio/_core/tests/test_run.py b/trio/_core/tests/test_run.py index afaf9e447c..75c1a10d6a 100644 --- a/trio/_core/tests/test_run.py +++ b/trio/_core/tests/test_run.py @@ -906,7 +906,6 @@ async def try_in_other_task(): async def test_cancel_branches(autojump_clock): - async def worker(depth, branch): with branch: if depth == 0: From 3bf48e070983f0badf2e9ec5ec2d000aa50be4c5 Mon Sep 17 00:00:00 2001 From: Joshua Oreman Date: Tue, 1 Jan 2019 22:33:57 -0800 Subject: [PATCH 03/12] attempt to fix coverage --- trio/_core/tests/test_run.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/trio/_core/tests/test_run.py b/trio/_core/tests/test_run.py index 75c1a10d6a..92ee287d34 100644 --- a/trio/_core/tests/test_run.py +++ b/trio/_core/tests/test_run.py @@ -888,7 +888,7 @@ async def sleep_until_cancelled(scope): with _core.CancelScope() as scope: with pytest.raises(RuntimeError) as exc_info: with scope: - pass + pass # pragma: no cover assert "may not be entered while it is already active; try" in str( exc_info.value ) @@ -897,7 +897,7 @@ async def sleep_until_cancelled(scope): async def try_in_other_task(): with pytest.raises(RuntimeError) as exc_info: with scope: - pass + pass # pragma: no cover assert "while it is already active in another task" in str( exc_info.value ) From bdc1577de04440455b34c8f8a45a28b6e039133c Mon Sep 17 00:00:00 2001 From: Joshua Oreman Date: Tue, 1 Jan 2019 22:58:34 -0800 Subject: [PATCH 04/12] Fix cancel-on-deadline-expiry which uses _cancel_no_notify directly + add a test for it --- trio/_core/_run.py | 33 ++++++++++++++++++--------------- trio/_core/tests/test_run.py | 19 +++++++++++++++++++ 2 files changed, 37 insertions(+), 15 deletions(-) diff --git a/trio/_core/_run.py b/trio/_core/_run.py index 7cd4828cb7..015fb7947c 100644 --- a/trio/_core/_run.py +++ b/trio/_core/_run.py @@ -179,12 +179,15 @@ class CancelScope: _tasks = attr.ib(factory=set, init=False) _scope_task = attr.ib(default=None, init=False) _effective_deadline = attr.ib(default=inf, init=False) - _branches = attr.ib(default=(), init=False) cancel_called = attr.ib(default=False, init=False) cancelled_caught = attr.ib(default=False, init=False) _deadline = attr.ib(default=inf, kw_only=True) _shield = attr.ib(default=False, kw_only=True) + # Most cancel scopes don't have branches; those that do will shadow + # this class-level empty tuple with an instance-level WeakSet + _branches = () + @enable_ki_protection def __enter__(self): task = _core.current_task() @@ -228,11 +231,12 @@ def __exit__(self, etype, exc, tb): value.__context__ = old_context def open_branch(self, *, deadline=inf, shield=None): - if not hasattr(self._branches, "add"): + if self._branches == (): # We keep _branches as an empty tuple until the first branch # is created, to avoid creating a fairly heavyweight # WeakSet on every cancel scope object - self._branches = weakref.WeakSet() + with self._might_change_effective_deadline(): + self._branches = weakref.WeakSet() if shield is None: shield = self._shield @@ -283,7 +287,10 @@ def _might_change_effective_deadline(self): yield finally: old = self._effective_deadline - if self.cancel_called or not self._tasks: + ever_had_branches = self._branches != () + if self.cancel_called or ( + not self._tasks and not ever_had_branches + ): new = inf else: new = self._deadline @@ -333,22 +340,18 @@ def _cancel_no_notify(self): if not self.cancel_called: with self._might_change_effective_deadline(): self.cancel_called = True - return self._tasks + affected_tasks = self._tasks + if self._branches: + affected_tasks = set(affected_tasks) + for branch in self._branches: + affected_tasks.update(branch._cancel_no_notify()) + return affected_tasks else: return set() @enable_ki_protection def cancel(self): - # Set cancel_called and collect affected tasks for us and all - # branches, then check cancellations for each affected task. - # This ordering ensures that each task receives the - # cancellation for its outermost cancelled scope. - affected_tasks = self._cancel_no_notify() - if self._branches: - affected_tasks = set(affected_tasks) - for branch in self.branches: - affected_tasks.update(branch._cancel_no_notify()) - for task in affected_tasks: + for task in self._cancel_no_notify(): task._attempt_delivery_of_any_pending_cancel() def _add_task(self, task): diff --git a/trio/_core/tests/test_run.py b/trio/_core/tests/test_run.py index 92ee287d34..6c1a6a98b3 100644 --- a/trio/_core/tests/test_run.py +++ b/trio/_core/tests/test_run.py @@ -934,6 +934,25 @@ async def worker(depth, branch): assert not any(branch.cancelled_caught for branch in all_branches) assert all(branch.cancel_called for branch in all_branches) + # Test cancellation of branches via deadline expiry, and with the + # top-level scope not itself bound to any work + root = _core.CancelScope(deadline=_core.current_time() + 0.5) + with fail_after(1): + async with _core.open_nursery() as nursery: + toplevel_branches = [] + for _ in range(3): + toplevel_branches.append(root.open_branch()) + nursery.start_soon(worker, 2, toplevel_branches[-1]) + await wait_all_tasks_blocked() + all_branches = list(root.branches) + assert root.cancel_called + assert not root.cancelled_caught + assert all(branch.cancel_called for branch in all_branches) + assert all( + branch.cancelled_caught == (branch in toplevel_branches) + for branch in all_branches + ) + # Branches can have their own deadline with _core.CancelScope(deadline=_core.current_time() + 2) as root: with root.open_branch() as branch: From 28e440af254b983151baea9460c56da25ec40feb Mon Sep 17 00:00:00 2001 From: Joshua Oreman Date: Tue, 1 Jan 2019 23:33:42 -0800 Subject: [PATCH 05/12] fix semi-spurious partial branch coverage in test --- trio/_core/tests/test_run.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/trio/_core/tests/test_run.py b/trio/_core/tests/test_run.py index 6c1a6a98b3..c124faf38f 100644 --- a/trio/_core/tests/test_run.py +++ b/trio/_core/tests/test_run.py @@ -948,10 +948,8 @@ async def worker(depth, branch): assert root.cancel_called assert not root.cancelled_caught assert all(branch.cancel_called for branch in all_branches) - assert all( - branch.cancelled_caught == (branch in toplevel_branches) - for branch in all_branches - ) + for branch in all_branches: + assert branch.cancelled_caught == (branch in toplevel_branches) # Branches can have their own deadline with _core.CancelScope(deadline=_core.current_time() + 2) as root: From d4066d7ade70d405ef29e854a005c3aff8f5977c Mon Sep 17 00:00:00 2001 From: Joshua Oreman Date: Wed, 2 Jan 2019 13:37:29 -0800 Subject: [PATCH 06/12] update comment --- trio/_core/_run.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/trio/_core/_run.py b/trio/_core/_run.py index 015fb7947c..b49ed70e14 100644 --- a/trio/_core/_run.py +++ b/trio/_core/_run.py @@ -800,7 +800,8 @@ class Runner: # {(deadline, id(CancelScope)): CancelScope} # only contains scopes with non-infinite deadlines that are currently - # attached to at least one task + # attached to at least one task or also control the cancellation of other + # scopes deadlines = attr.ib(default=attr.Factory(SortedDict)) init_task = attr.ib(default=None) From cf2b3b5a5799a555ce28000abb082140044cddbf Mon Sep 17 00:00:00 2001 From: Joshua Oreman Date: Wed, 2 Jan 2019 22:04:52 -0800 Subject: [PATCH 07/12] s/branch/linked child/g --- docs/source/reference-core.rst | 51 +++++++++--------- newsfragments/607.feature.rst | 2 +- trio/_core/_run.py | 77 +++++++++++++------------- trio/_core/tests/test_run.py | 98 +++++++++++++++++----------------- 4 files changed, 118 insertions(+), 110 deletions(-) diff --git a/docs/source/reference-core.rst b/docs/source/reference-core.rst index 7c7d6592e2..ccf8fbe181 100644 --- a/docs/source/reference-core.rst +++ b/docs/source/reference-core.rst @@ -584,40 +584,43 @@ objects. cancelled, then :attr:`cancelled_caught` is usually more appropriate. - .. method:: open_branch(*, deadline=math.inf, shield=None) + .. method:: linked_child(*, deadline=math.inf, shield=None) Return another :class:`CancelScope` object that automatically becomes cancelled when this one does. We say that the returned - cancel scope is a "branch" of this "source" cancel scope. - - The relationship between the source cancel scope and the new - branch is one-way: if the source cancel scope becomes cancelled, - all of its branches do too, but any of the branches can - independently become cancelled without affecting the - source. Each branch has its own :attr:`deadline`, which is - :data:`math.inf` (not inherited from the source scope) if the - ``deadline`` argument is unspecified; the expiry of a branch's - deadline cancels that branch only. - - The new branch inherits its initial :attr:`shield` attribute - from the source, unless overridden via the ``shield`` argument. - The branch :attr:`shield` may be changed without affecting the - source, but changes to the source :attr:`shield` will be - propagated to all branches, overriding any local :attr:`shield` + cancel scope is a "linked child" of this "parent" cancel scope. + Note that this relationship has nothing to do with lexical + nesting of ``with`` blocks. + + The relationship between the parent cancel scope and its new + linked child is one-way: if the parent cancel scope becomes + cancelled, all of its linked children do too, but any of the + children can independently become cancelled without affecting + the source. Each child has its own :attr:`deadline`, which is + :data:`math.inf` (not inherited from the parent scope) if the + ``deadline`` argument is unspecified; the expiry of a linked + child's deadline cancels that child only. + + The new linked child inherits its initial :attr:`shield` attribute + from the parent, unless overridden via the ``shield`` argument. + The child :attr:`shield` may be changed without affecting the + parent, but changes to the parent :attr:`shield` will be + propagated to all children, overriding any local :attr:`shield` value they've previously set. Multiple layers of cancel scope linkage are supported. The - cancellation of any source scope affects all its branches, all - their branches, and so on. + cancellation of any parent scope affects all its linked + children, all their linked children, and so on. - .. attribute:: branches + .. attribute:: linked_children An iterable yielding all the other cancel scopes that will become cancelled when this one does. That includes all the - branches of this cancel scope and all of their branches, recursively. - Cancel scopes track their branches by weak reference, so a scope - may no longer be reflected in :attr:`branches` once it has no other - references active. + cancel scopes created by calls to :meth:`linked_child` on this + cancel scope and all of its children, recursively. Cancel + scopes track their linked children by weak reference, so a scope + may no longer be reflected in :attr:`linked_children` once it + has no other references active. Trio also provides several convenience functions for the common diff --git a/newsfragments/607.feature.rst b/newsfragments/607.feature.rst index e29d4ddd83..f7d635e99b 100644 --- a/newsfragments/607.feature.rst +++ b/newsfragments/607.feature.rst @@ -3,4 +3,4 @@ Add support for "unbound cancel scopes": you can now construct a can pass it to another task which will use it to wrap some work that you want to be able to cancel from afar. Add the ability to create a derived :class:`~trio.CancelScope` that will become cancelled when its -parent does, using :meth:`~trio.CancelScope.open_branch`. +parent does, using :meth:`~trio.CancelScope.linked_child`. diff --git a/trio/_core/_run.py b/trio/_core/_run.py index b49ed70e14..4c722639ab 100644 --- a/trio/_core/_run.py +++ b/trio/_core/_run.py @@ -118,7 +118,7 @@ def deadline_to_sleep_time(self, deadline): ################################################################ -@attr.s(cmp=False, repr=False) +@attr.s(cmp=False, repr=False, slots=True) class CancelScope: """A *cancellation scope*: the link between a unit of cancellable work and Trio's cancellation system. @@ -167,7 +167,7 @@ class CancelScope: one is still active. (You'll get a :exc:`RuntimeError` if you try.) If you want multiple blocks of work to be cancelled with the same call to :meth:`cancel` or at the expiry of the same - deadline, see the :meth:`open_branch` method. + deadline, see the :meth:`linked_child` method. The :class:`CancelScope` constructor takes initial values for the cancel scope's :attr:`deadline` and :attr:`shield` attributes; these @@ -181,21 +181,23 @@ class CancelScope: _effective_deadline = attr.ib(default=inf, init=False) cancel_called = attr.ib(default=False, init=False) cancelled_caught = attr.ib(default=False, init=False) + + # Most cancel scopes don't have linked children; those that do + # will replace this empty tuple with a WeakSet + _linked_children = attr.ib(default=(), init=False) + + # Constructor arguments: _deadline = attr.ib(default=inf, kw_only=True) _shield = attr.ib(default=False, kw_only=True) - # Most cancel scopes don't have branches; those that do will shadow - # this class-level empty tuple with an instance-level WeakSet - _branches = () - @enable_ki_protection def __enter__(self): task = _core.current_task() if self._scope_task is not None: raise RuntimeError( "cancel scope may not be entered while it is already " - "active{}; try `with cancel_scope.branch() as new_scope:` " - "instead".format( + "active{}; try `with cancel_scope.linked_child() as " + "new_scope:` instead".format( "" if self._scope_task is task else " in another task ({!r})".format(self._scope_task.name) ) @@ -230,27 +232,27 @@ def __exit__(self, etype, exc, tb): assert value is remaining_error_after_cancel_scope value.__context__ = old_context - def open_branch(self, *, deadline=inf, shield=None): - if self._branches == (): - # We keep _branches as an empty tuple until the first branch - # is created, to avoid creating a fairly heavyweight - # WeakSet on every cancel scope object + def linked_child(self, *, deadline=inf, shield=None): + if self._linked_children == (): + # We keep _linked_children as an empty tuple until the + # first linked_child is created, to avoid creating a + # fairly heavyweight WeakSet on every cancel scope object with self._might_change_effective_deadline(): - self._branches = weakref.WeakSet() + self._linked_children = weakref.WeakSet() if shield is None: shield = self._shield child = CancelScope(deadline=deadline, shield=shield) if self.cancel_called: child.cancel() - self._branches.add(child) + self._linked_children.add(child) return child @property - def branches(self): - for branch in self._branches: - yield branch - yield from branch.branches + def linked_children(self): + for child in self._linked_children: + yield child + yield from child.linked_children def __repr__(self): if self._scope_task is None: @@ -271,13 +273,16 @@ def __repr__(self): else: state = ", cancel in {:.2f}sec".format(self.deadline - now) - if self._branches: - branches = ", {} branches".format(sum(1 for _ in self.branches)) + if self._linked_children: + count = sum(1 for _ in self.linked_children) + linked_children = ", {} linked child{}".format( + count, "ren" if count > 1 else "" + ) else: - branches = "" + linked_children = "" return "".format( - id(self), binding, state, branches + id(self), binding, state, linked_children ) @contextmanager @@ -287,9 +292,9 @@ def _might_change_effective_deadline(self): yield finally: old = self._effective_deadline - ever_had_branches = self._branches != () + ever_had_linked_children = self._linked_children != () if self.cancel_called or ( - not self._tasks and not ever_had_branches + not self._tasks and not ever_had_linked_children ): new = inf else: @@ -321,18 +326,18 @@ def shield(self, new_value): if not isinstance(new_value, bool): raise TypeError("shield must be a bool") - # Set shielding for us and all branches, then check cancellations - # for us and all branches if shielding was turned off. This ordering - # ensures that each task receives the cancellation for its - # outermost cancelled scope. + # Set shielding for us and all linked children, then check + # cancellations for us and all linked children if shielding + # was turned off. This ordering ensures that each task + # receives the cancellation for its outermost cancelled scope. self._shield = new_value - for branch in self.branches: - branch._shield = new_value + for child in self.linked_children: + child._shield = new_value if not self._shield: for task in self._tasks: task._attempt_delivery_of_any_pending_cancel() - for branch in self.branches: - for task in branch._tasks: + for child in self.linked_children: + for task in child._tasks: task._attempt_delivery_of_any_pending_cancel() def _cancel_no_notify(self): @@ -341,10 +346,10 @@ def _cancel_no_notify(self): with self._might_change_effective_deadline(): self.cancel_called = True affected_tasks = self._tasks - if self._branches: + if self._linked_children: affected_tasks = set(affected_tasks) - for branch in self._branches: - affected_tasks.update(branch._cancel_no_notify()) + for child in self._linked_children: + affected_tasks.update(child._cancel_no_notify()) return affected_tasks else: return set() diff --git a/trio/_core/tests/test_run.py b/trio/_core/tests/test_run.py index c124faf38f..84e0726ea8 100644 --- a/trio/_core/tests/test_run.py +++ b/trio/_core/tests/test_run.py @@ -556,8 +556,8 @@ async def test_cancel_scope_repr(autojump_clock): assert "cancel soon" in repr(scope) scope.cancel() assert "cancelled" in repr(scope) - with scope.open_branch(): - assert ", 1 branches" in repr(scope) + with scope.linked_child(): + assert ", 1 linked child" in repr(scope) def test_cancel_points(): @@ -905,96 +905,96 @@ async def try_in_other_task(): nursery.start_soon(try_in_other_task) -async def test_cancel_branches(autojump_clock): - async def worker(depth, branch): - with branch: +async def test_cancel_linked_children(autojump_clock): + async def worker(depth, child): + with child: if depth == 0: await sleep_forever() else: async with _core.open_nursery() as nursery: - nursery.start_soon(worker, depth - 1, branch.open_branch()) - nursery.start_soon(worker, depth - 1, branch.open_branch()) - await worker(depth - 1, branch.open_branch()) + nursery.start_soon(worker, depth - 1, child.linked_child()) + nursery.start_soon(worker, depth - 1, child.linked_child()) + await worker(depth - 1, child.linked_child()) with _core.CancelScope() as root, fail_after(1): async with _core.open_nursery() as nursery: - nursery.start_soon(worker, 3, root.open_branch()) + nursery.start_soon(worker, 3, root.linked_child()) await wait_all_tasks_blocked() - all_branches = list(root.branches) - # One branch created from the open_branch() call in this block + all_children = list(root.linked_children) + # One child created from the linked_child() call in this block # Three from the single worker(3) # Three from each of three worker(2) = 9 total # Three from each of nine worker(1) = 27 total - assert len(all_branches) == 1 + 3 + 9 + 27 + assert len(all_children) == 1 + 3 + 9 + 27 root.cancel() # Test the logic to delay notifying any tasks affected by a cancel() # until all cancel_called members are updated assert root.cancelled_caught and root.cancel_called - assert not any(branch.cancelled_caught for branch in all_branches) - assert all(branch.cancel_called for branch in all_branches) + assert not any(child.cancelled_caught for child in all_children) + assert all(child.cancel_called for child in all_children) - # Test cancellation of branches via deadline expiry, and with the - # top-level scope not itself bound to any work + # Test cancellation of linked children via deadline expiry, and + # with the top-level scope not itself bound to any work root = _core.CancelScope(deadline=_core.current_time() + 0.5) with fail_after(1): async with _core.open_nursery() as nursery: - toplevel_branches = [] + toplevel_children = [] for _ in range(3): - toplevel_branches.append(root.open_branch()) - nursery.start_soon(worker, 2, toplevel_branches[-1]) + toplevel_children.append(root.linked_child()) + nursery.start_soon(worker, 2, toplevel_children[-1]) await wait_all_tasks_blocked() - all_branches = list(root.branches) + all_children = list(root.linked_children) assert root.cancel_called assert not root.cancelled_caught - assert all(branch.cancel_called for branch in all_branches) - for branch in all_branches: - assert branch.cancelled_caught == (branch in toplevel_branches) + assert all(child.cancel_called for child in all_children) + for child in all_children: + assert child.cancelled_caught == (child in toplevel_children) - # Branches can have their own deadline + # Childes can have their own deadline with _core.CancelScope(deadline=_core.current_time() + 2) as root: - with root.open_branch() as branch: - branch.deadline = _core.current_time() + 1 - assert root.deadline == pytest.approx(branch.deadline + 1) + with root.linked_child() as child: + child.deadline = _core.current_time() + 1 + assert root.deadline == pytest.approx(child.deadline + 1) await sleep_forever() - assert branch.cancel_called and branch.cancelled_caught + assert child.cancel_called and child.cancelled_caught assert not root.cancel_called and not root.cancelled_caught - # Branches inherit shielding from the parent, can change independently - # but parent's later changes will override + # Linked children inherit shielding from the parent, can change + # independently but parent's later changes will override with _core.CancelScope() as root: - with root.open_branch() as branch: - assert not root.shield and not branch.shield - assert root.open_branch(shield=True).shield - branch.shield = True - assert not root.shield and branch.shield + with root.linked_child() as child: + assert not root.shield and not child.shield + assert root.linked_child(shield=True).shield + child.shield = True + assert not root.shield and child.shield root.shield = False - assert not root.shield and not branch.shield + assert not root.shield and not child.shield root.shield = True - assert root.shield and branch.shield - branch.shield = False - assert root.shield and not branch.shield - assert root.open_branch().shield - assert not root.open_branch(shield=False).shield + assert root.shield and child.shield + child.shield = False + assert root.shield and not child.shield + assert root.linked_child().shield + assert not root.linked_child(shield=False).shield - # Unshielding occurs simultaneously for all branches affected by the - # .shield = False operation + # Unshielding occurs simultaneously for all linked children + # affected by the .shield = False operation shielded = _core.CancelScope(shield=True) cancelled = _core.CancelScope() cancelled.cancel() async def do_shield_order_test(outer_shield, inner_shield): - with cancelled.open_branch() as outer_cancelled: - with outer_shield.open_branch(): - with cancelled.open_branch() as inner_cancelled: - with inner_shield.open_branch(): + with cancelled.linked_child() as outer_cancelled: + with outer_shield.linked_child(): + with cancelled.linked_child() as inner_cancelled: + with inner_shield.linked_child(): await sleep_forever() assert outer_cancelled.cancelled_caught assert not inner_cancelled.cancelled_caught - shield1 = shielded.open_branch() - shield2 = shielded.open_branch() + shield1 = shielded.linked_child() + shield2 = shielded.linked_child() async with _core.open_nursery() as nursery: nursery.start_soon(do_shield_order_test, shield1, shield2) From 29ec0ad9f48900ac701bccd4f289a115999fd3a3 Mon Sep 17 00:00:00 2001 From: Joshua Oreman Date: Sat, 19 Jan 2019 15:24:05 -0800 Subject: [PATCH 08/12] Remove linked children from this PR --- docs/source/reference-core.rst | 38 -------- newsfragments/607.feature.rst | 4 +- notes-to-self/graceful-shutdown-idea.py | 21 +++-- trio/_core/_run.py | 68 ++------------ trio/_core/tests/test_run.py | 114 +----------------------- 5 files changed, 22 insertions(+), 223 deletions(-) diff --git a/docs/source/reference-core.rst b/docs/source/reference-core.rst index 6994e8eb3b..c2e0372eca 100644 --- a/docs/source/reference-core.rst +++ b/docs/source/reference-core.rst @@ -584,44 +584,6 @@ objects. cancelled, then :attr:`cancelled_caught` is usually more appropriate. - .. method:: linked_child(*, deadline=math.inf, shield=None) - - Return another :class:`CancelScope` object that automatically - becomes cancelled when this one does. We say that the returned - cancel scope is a "linked child" of this "parent" cancel scope. - Note that this relationship has nothing to do with lexical - nesting of ``with`` blocks. - - The relationship between the parent cancel scope and its new - linked child is one-way: if the parent cancel scope becomes - cancelled, all of its linked children do too, but any of the - children can independently become cancelled without affecting - the source. Each child has its own :attr:`deadline`, which is - :data:`math.inf` (not inherited from the parent scope) if the - ``deadline`` argument is unspecified; the expiry of a linked - child's deadline cancels that child only. - - The new linked child inherits its initial :attr:`shield` attribute - from the parent, unless overridden via the ``shield`` argument. - The child :attr:`shield` may be changed without affecting the - parent, but changes to the parent :attr:`shield` will be - propagated to all children, overriding any local :attr:`shield` - value they've previously set. - - Multiple layers of cancel scope linkage are supported. The - cancellation of any parent scope affects all its linked - children, all their linked children, and so on. - - .. attribute:: linked_children - - An iterable yielding all the other cancel scopes that will - become cancelled when this one does. That includes all the - cancel scopes created by calls to :meth:`linked_child` on this - cancel scope and all of its children, recursively. Cancel - scopes track their linked children by weak reference, so a scope - may no longer be reflected in :attr:`linked_children` once it - has no other references active. - Trio also provides several convenience functions for the common situation of just wanting to impose a timeout on some code: diff --git a/newsfragments/607.feature.rst b/newsfragments/607.feature.rst index f7d635e99b..35fdf81d58 100644 --- a/newsfragments/607.feature.rst +++ b/newsfragments/607.feature.rst @@ -1,6 +1,4 @@ Add support for "unbound cancel scopes": you can now construct a :class:`trio.CancelScope` without entering its context, e.g. so you can pass it to another task which will use it to wrap some work that -you want to be able to cancel from afar. Add the ability to create a -derived :class:`~trio.CancelScope` that will become cancelled when its -parent does, using :meth:`~trio.CancelScope.linked_child`. +you want to be able to cancel from afar. diff --git a/notes-to-self/graceful-shutdown-idea.py b/notes-to-self/graceful-shutdown-idea.py index fa0c06d6b1..2477596f72 100644 --- a/notes-to-self/graceful-shutdown-idea.py +++ b/notes-to-self/graceful-shutdown-idea.py @@ -3,17 +3,24 @@ class GracefulShutdownManager: def __init__(self): - self._root_scope = trio.CancelScope() + self._shutting_down = False + self._cancel_scopes = set() def start_shutdown(self): - self._root_scope.cancel() + self._shutting_down = True + for cancel_scope in self._cancel_scopes: + cancel_scope.cancel() def cancel_on_graceful_shutdown(self): - return self._root_scope.open_branch() + cancel_scope = trio.CancelScope() + self._cancel_scopes.add(cancel_scope) + if self._shutting_down: + cancel_scope.cancel() + return cancel_scope @property def shutting_down(self): - return self._root_scope.cancel_called + return self._shutting_down # Code can check gsm.shutting_down occasionally at appropriate points to see # if it should exit. @@ -30,8 +37,8 @@ async def stream_handler(stream): # To trigger the shutdown: async def listen_for_shutdown_signals(): - with trio.catch_signals({signal.SIGINT, signal.SIGTERM}) as signal_aiter: - async for batch in signal_aiter: + with trio.open_signal_receiver(signal.SIGINT, signal.SIGTERM) as signal_aiter: + async for sig in signal_aiter: gsm.start_shutdown() break # TODO: it'd be nice to have some logic like "if we get another @@ -39,7 +46,7 @@ async def listen_for_shutdown_signals(): # That's easy enough: # # with trio.move_on_after(30): - # async for batch in signal_aiter: + # async for sig in signal_aiter: # break # sys.exit() # diff --git a/trio/_core/_run.py b/trio/_core/_run.py index 4aba87c5eb..71624deee3 100644 --- a/trio/_core/_run.py +++ b/trio/_core/_run.py @@ -164,10 +164,7 @@ class CancelScope: Cancel scopes are *not* reentrant: you can't enter a second ``with`` block using the same :class:`CancelScope` while the first - one is still active. (You'll get a :exc:`RuntimeError` if you - try.) If you want multiple blocks of work to be cancelled with - the same call to :meth:`cancel` or at the expiry of the same - deadline, see the :meth:`linked_child` method. + one is still active. (You'll get a :exc:`RuntimeError` if you try.) The :class:`CancelScope` constructor takes initial values for the cancel scope's :attr:`deadline` and :attr:`shield` attributes; these @@ -182,10 +179,6 @@ class CancelScope: cancel_called = attr.ib(default=False, init=False) cancelled_caught = attr.ib(default=False, init=False) - # Most cancel scopes don't have linked children; those that do - # will replace this empty tuple with a WeakSet - _linked_children = attr.ib(default=(), init=False) - # Constructor arguments: _deadline = attr.ib(default=inf, kw_only=True) _shield = attr.ib(default=False, kw_only=True) @@ -196,8 +189,7 @@ def __enter__(self): if self._scope_task is not None: raise RuntimeError( "cancel scope may not be entered while it is already " - "active{}; try `with cancel_scope.linked_child() as " - "new_scope:` instead".format( + "active{}".format( "" if self._scope_task is task else " in another task ({!r})".format(self._scope_task.name) ) @@ -232,28 +224,6 @@ def __exit__(self, etype, exc, tb): assert value is remaining_error_after_cancel_scope value.__context__ = old_context - def linked_child(self, *, deadline=inf, shield=None): - if self._linked_children == (): - # We keep _linked_children as an empty tuple until the - # first linked_child is created, to avoid creating a - # fairly heavyweight WeakSet on every cancel scope object - with self._might_change_effective_deadline(): - self._linked_children = weakref.WeakSet() - - if shield is None: - shield = self._shield - child = CancelScope(deadline=deadline, shield=shield) - if self.cancel_called: - child.cancel() - self._linked_children.add(child) - return child - - @property - def linked_children(self): - for child in self._linked_children: - yield child - yield from child.linked_children - def __repr__(self): if self._scope_task is None: binding = "unbound" @@ -273,16 +243,8 @@ def __repr__(self): else: state = ", cancel in {:.2f}sec".format(self.deadline - now) - if self._linked_children: - count = sum(1 for _ in self.linked_children) - linked_children = ", {} linked child{}".format( - count, "ren" if count > 1 else "" - ) - else: - linked_children = "" - - return "".format( - id(self), binding, state, linked_children + return "".format( + id(self), binding, state ) @contextmanager @@ -292,10 +254,7 @@ def _might_change_effective_deadline(self): yield finally: old = self._effective_deadline - ever_had_linked_children = self._linked_children != () - if self.cancel_called or ( - not self._tasks and not ever_had_linked_children - ): + if self.cancel_called or not self._tasks: new = inf else: new = self._deadline @@ -325,32 +284,17 @@ def shield(self): def shield(self, new_value): if not isinstance(new_value, bool): raise TypeError("shield must be a bool") - - # Set shielding for us and all linked children, then check - # cancellations for us and all linked children if shielding - # was turned off. This ordering ensures that each task - # receives the cancellation for its outermost cancelled scope. self._shield = new_value - for child in self.linked_children: - child._shield = new_value if not self._shield: for task in self._tasks: task._attempt_delivery_of_any_pending_cancel() - for child in self.linked_children: - for task in child._tasks: - task._attempt_delivery_of_any_pending_cancel() def _cancel_no_notify(self): # returns the affected tasks if not self.cancel_called: with self._might_change_effective_deadline(): self.cancel_called = True - affected_tasks = self._tasks - if self._linked_children: - affected_tasks = set(affected_tasks) - for child in self._linked_children: - affected_tasks.update(child._cancel_no_notify()) - return affected_tasks + return self._tasks else: return set() diff --git a/trio/_core/tests/test_run.py b/trio/_core/tests/test_run.py index 84e0726ea8..8047f59caf 100644 --- a/trio/_core/tests/test_run.py +++ b/trio/_core/tests/test_run.py @@ -556,8 +556,6 @@ async def test_cancel_scope_repr(autojump_clock): assert "cancel soon" in repr(scope) scope.cancel() assert "cancelled" in repr(scope) - with scope.linked_child(): - assert ", 1 linked child" in repr(scope) def test_cancel_points(): @@ -889,7 +887,7 @@ async def sleep_until_cancelled(scope): with pytest.raises(RuntimeError) as exc_info: with scope: pass # pragma: no cover - assert "may not be entered while it is already active; try" in str( + assert "may not be entered while it is already active" in str( exc_info.value ) async with _core.open_nursery() as nursery: @@ -905,116 +903,6 @@ async def try_in_other_task(): nursery.start_soon(try_in_other_task) -async def test_cancel_linked_children(autojump_clock): - async def worker(depth, child): - with child: - if depth == 0: - await sleep_forever() - else: - async with _core.open_nursery() as nursery: - nursery.start_soon(worker, depth - 1, child.linked_child()) - nursery.start_soon(worker, depth - 1, child.linked_child()) - await worker(depth - 1, child.linked_child()) - - with _core.CancelScope() as root, fail_after(1): - async with _core.open_nursery() as nursery: - nursery.start_soon(worker, 3, root.linked_child()) - await wait_all_tasks_blocked() - all_children = list(root.linked_children) - # One child created from the linked_child() call in this block - # Three from the single worker(3) - # Three from each of three worker(2) = 9 total - # Three from each of nine worker(1) = 27 total - assert len(all_children) == 1 + 3 + 9 + 27 - root.cancel() - - # Test the logic to delay notifying any tasks affected by a cancel() - # until all cancel_called members are updated - assert root.cancelled_caught and root.cancel_called - assert not any(child.cancelled_caught for child in all_children) - assert all(child.cancel_called for child in all_children) - - # Test cancellation of linked children via deadline expiry, and - # with the top-level scope not itself bound to any work - root = _core.CancelScope(deadline=_core.current_time() + 0.5) - with fail_after(1): - async with _core.open_nursery() as nursery: - toplevel_children = [] - for _ in range(3): - toplevel_children.append(root.linked_child()) - nursery.start_soon(worker, 2, toplevel_children[-1]) - await wait_all_tasks_blocked() - all_children = list(root.linked_children) - assert root.cancel_called - assert not root.cancelled_caught - assert all(child.cancel_called for child in all_children) - for child in all_children: - assert child.cancelled_caught == (child in toplevel_children) - - # Childes can have their own deadline - with _core.CancelScope(deadline=_core.current_time() + 2) as root: - with root.linked_child() as child: - child.deadline = _core.current_time() + 1 - assert root.deadline == pytest.approx(child.deadline + 1) - await sleep_forever() - assert child.cancel_called and child.cancelled_caught - assert not root.cancel_called and not root.cancelled_caught - - # Linked children inherit shielding from the parent, can change - # independently but parent's later changes will override - with _core.CancelScope() as root: - with root.linked_child() as child: - assert not root.shield and not child.shield - assert root.linked_child(shield=True).shield - child.shield = True - assert not root.shield and child.shield - root.shield = False - assert not root.shield and not child.shield - root.shield = True - assert root.shield and child.shield - child.shield = False - assert root.shield and not child.shield - assert root.linked_child().shield - assert not root.linked_child(shield=False).shield - - # Unshielding occurs simultaneously for all linked children - # affected by the .shield = False operation - - shielded = _core.CancelScope(shield=True) - cancelled = _core.CancelScope() - cancelled.cancel() - - async def do_shield_order_test(outer_shield, inner_shield): - with cancelled.linked_child() as outer_cancelled: - with outer_shield.linked_child(): - with cancelled.linked_child() as inner_cancelled: - with inner_shield.linked_child(): - await sleep_forever() - assert outer_cancelled.cancelled_caught - assert not inner_cancelled.cancelled_caught - - shield1 = shielded.linked_child() - shield2 = shielded.linked_child() - - async with _core.open_nursery() as nursery: - nursery.start_soon(do_shield_order_test, shield1, shield2) - nursery.start_soon(do_shield_order_test, shield2, shield1) - await wait_all_tasks_blocked() - shielded.shield = False - - # How it works: If unshielding didn't occur simultaneously, then - # either shield1 would be unshielded before shield2 or vice versa. - # That would mean one of the do_shield_order_test tasks would - # momentarily have a cancel stack like: - # outer_cancelled: cancelled - # outer_shield: shielded - # inner_cancelled: cancelled - # inner_shield: not shielded - # and it would be delivered a cancellation for inner_cancelled. - # We assert that both tasks receive cancellations for outer_cancelled, - # which could only happen if the unshielding happens simultaneously. - - async def test_timekeeping(): # probably a good idea to use a real clock for *one* test anyway... TARGET = 0.1 From fc19347cf24f1563eb92545179856af1d8441822 Mon Sep 17 00:00:00 2001 From: Joshua Oreman Date: Sat, 19 Jan 2019 15:37:05 -0800 Subject: [PATCH 09/12] respond to CR comments --- trio/_core/_run.py | 19 +++++++++++++------ trio/_core/tests/test_run.py | 13 ++++++++----- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/trio/_core/_run.py b/trio/_core/_run.py index 71624deee3..c6d994a5b1 100644 --- a/trio/_core/_run.py +++ b/trio/_core/_run.py @@ -230,18 +230,24 @@ def __repr__(self): else: binding = "bound to {!r}".format(self._scope_task.name) if len(self._tasks) > 1: - binding += " and its {} children".format(len(self._tasks) - 1) + binding += " and its {} descendant{}".format( + len(self._tasks) - 1, "s" if len(self._tasks) > 2 else "" + ) if self.cancel_called: state = ", cancelled" elif self.deadline == inf: state = "" else: - now = current_time() - if now >= self.deadline: - state = ", cancel soon" + try: + now = current_time() + except RuntimeError: # must be called from async context + state = "" else: - state = ", cancel in {:.2f}sec".format(self.deadline - now) + state = ", deadline is {:.2f} seconds {}".format( + abs(self.deadline - now), + "from now" if self.deadline >= now else "ago" + ) return "".format( id(self), binding, state @@ -450,7 +456,8 @@ class NurseryManager: @enable_ki_protection async def __aenter__(self): - self._scope = CancelScope().__enter__() + self._scope = CancelScope() + self._scope.__enter__() self._nursery = Nursery(current_task(), self._scope) return self._nursery diff --git a/trio/_core/tests/test_run.py b/trio/_core/tests/test_run.py index 8047f59caf..394197180e 100644 --- a/trio/_core/tests/test_run.py +++ b/trio/_core/tests/test_run.py @@ -18,6 +18,7 @@ from .tutil import check_sequence_matches, gc_collect_harder from ... import _core +from ..._threads import run_sync_in_worker_thread from ..._timeouts import sleep, fail_after from ..._util import aiter_compat from ...testing import ( @@ -541,19 +542,21 @@ async def main(): assert "Instrument has been disabled" in caplog.records[0].message -async def test_cancel_scope_repr(autojump_clock): +async def test_cancel_scope_repr(mock_clock): scope = _core.CancelScope() assert "unbound" in repr(scope) with scope: assert "bound to {!r}".format(_core.current_task().name) in repr(scope) async with _core.open_nursery() as nursery: nursery.start_soon(sleep, 10) - assert "and its 1 children" in repr(scope) + assert "and its 1 descendant" in repr(scope) nursery.cancel_scope.cancel() - scope.deadline = _core.current_time() + 10 - assert "cancel in 10.00sec" in repr(scope) scope.deadline = _core.current_time() - 1 - assert "cancel soon" in repr(scope) + assert "deadline is 1.00 seconds ago" in repr(scope) + scope.deadline = _core.current_time() + 10 + assert "deadline is 10.00 seconds from now" in repr(scope) + # when not in async context, can't get the current time + assert "deadline" not in await run_sync_in_worker_thread(repr, scope) scope.cancel() assert "cancelled" in repr(scope) From 360dcaeefc814d5edc479610d84edc68a98aef02 Mon Sep 17 00:00:00 2001 From: Joshua Oreman Date: Sat, 19 Jan 2019 16:00:10 -0800 Subject: [PATCH 10/12] remove a few other things that were only used by the linked child code --- trio/_core/_run.py | 4 +--- trio/_core/tests/test_run.py | 1 - 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/trio/_core/_run.py b/trio/_core/_run.py index c6d994a5b1..27d8a90eab 100644 --- a/trio/_core/_run.py +++ b/trio/_core/_run.py @@ -5,7 +5,6 @@ import select import sys import threading -import weakref from collections import deque import collections.abc from contextlib import contextmanager, closing @@ -755,8 +754,7 @@ class Runner: # {(deadline, id(CancelScope)): CancelScope} # only contains scopes with non-infinite deadlines that are currently - # attached to at least one task or also control the cancellation of other - # scopes + # attached to at least one task deadlines = attr.ib(default=attr.Factory(SortedDict)) init_task = attr.ib(default=None) diff --git a/trio/_core/tests/test_run.py b/trio/_core/tests/test_run.py index 394197180e..0c8a511e49 100644 --- a/trio/_core/tests/test_run.py +++ b/trio/_core/tests/test_run.py @@ -1,7 +1,6 @@ import contextvars import functools import platform -import re import sys import threading import time From 34ba6283903fecd0d641fe52b74793c1c1532545 Mon Sep 17 00:00:00 2001 From: Joshua Oreman Date: Tue, 22 Jan 2019 16:41:19 -0800 Subject: [PATCH 11/12] trivial change to poke CI --- newsfragments/607.feature.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/newsfragments/607.feature.rst b/newsfragments/607.feature.rst index 35fdf81d58..db78a2ea4c 100644 --- a/newsfragments/607.feature.rst +++ b/newsfragments/607.feature.rst @@ -1,4 +1,4 @@ Add support for "unbound cancel scopes": you can now construct a -:class:`trio.CancelScope` without entering its context, e.g. so you +:class:`trio.CancelScope` without entering its context, e.g., so you can pass it to another task which will use it to wrap some work that you want to be able to cancel from afar. From 5f96b786c300698a42c4d17c0622177a6cec082f Mon Sep 17 00:00:00 2001 From: Joshua Oreman Date: Sun, 27 Jan 2019 18:47:48 -0800 Subject: [PATCH 12/12] CR comments --- docs/source/reference-core.rst | 57 +++------------------------------- trio/_core/_run.py | 48 +++++++++++++++++++++++++++- trio/_core/tests/test_run.py | 24 ++++++++------ 3 files changed, 67 insertions(+), 62 deletions(-) diff --git a/docs/source/reference-core.rst b/docs/source/reference-core.rst index c2e0372eca..a1d3a83389 100644 --- a/docs/source/reference-core.rst +++ b/docs/source/reference-core.rst @@ -500,58 +500,11 @@ objects. .. autoclass:: trio.CancelScope - .. attribute:: deadline - - Read-write, :class:`float`. An absolute time on the current - run's clock at which this scope will automatically become - cancelled. You can adjust the deadline by modifying this - attribute, e.g.:: - - # I need a little more time! - cancel_scope.deadline += 30 - - Note that for efficiency, the core run loop only checks for - expired deadlines every once in a while. This means that in - certain cases there may be a short delay between when the clock - says the deadline should have expired, and when checkpoints - start raising :exc:`~trio.Cancelled`. This is a very obscure - corner case that you're unlikely to notice, but we document it - for completeness. (If this *does* cause problems for you, of - course, then `we want to know! - `__) - - Defaults to :data:`math.inf`, which means "no deadline", though - this can be overridden by the ``deadline=`` argument to - the :class:`~trio.CancelScope` constructor. - - .. attribute:: shield - - Read-write, :class:`bool`, default :data:`False`. So long as - this is set to :data:`True`, then the code inside this scope - will not receive :exc:`~trio.Cancelled` exceptions from scopes - that are outside this scope. They can still receive - :exc:`~trio.Cancelled` exceptions from (1) this scope, or (2) - scopes inside this scope. You can modify this attribute:: - - with trio.CancelScope() as cancel_scope: - cancel_scope.shield = True - # This cannot be interrupted by any means short of - # killing the process: - await sleep(10) - - cancel_scope.shield = False - # Now this can be cancelled normally: - await sleep(10) - - Defaults to :data:`False`, though this can be overridden by the - ``shield=`` argument to the :class:`~trio.CancelScope` constructor. - - .. method:: cancel() - - Cancels this scope immediately. - - This method is idempotent, i.e. if the scope was already - cancelled then this method silently does nothing. + .. autoattribute:: deadline + + .. autoattribute:: shield + + .. automethod:: cancel() .. attribute:: cancelled_caught diff --git a/trio/_core/_run.py b/trio/_core/_run.py index 27d8a90eab..f96f6c9181 100644 --- a/trio/_core/_run.py +++ b/trio/_core/_run.py @@ -169,7 +169,6 @@ class CancelScope: cancel scope's :attr:`deadline` and :attr:`shield` attributes; these may be freely modified after construction, whether or not the scope has been entered yet, and changes take immediate effect. - """ _tasks = attr.ib(factory=set, init=False) @@ -273,6 +272,28 @@ def _might_change_effective_deadline(self): @property def deadline(self): + """Read-write, :class:`float`. An absolute time on the current + run's clock at which this scope will automatically become + cancelled. You can adjust the deadline by modifying this + attribute, e.g.:: + + # I need a little more time! + cancel_scope.deadline += 30 + + Note that for efficiency, the core run loop only checks for + expired deadlines every once in a while. This means that in + certain cases there may be a short delay between when the clock + says the deadline should have expired, and when checkpoints + start raising :exc:`~trio.Cancelled`. This is a very obscure + corner case that you're unlikely to notice, but we document it + for completeness. (If this *does* cause problems for you, of + course, then `we want to know! + `__) + + Defaults to :data:`math.inf`, which means "no deadline", though + this can be overridden by the ``deadline=`` argument to + the :class:`~trio.CancelScope` constructor. + """ return self._deadline @deadline.setter @@ -282,6 +303,26 @@ def deadline(self, new_deadline): @property def shield(self): + """Read-write, :class:`bool`, default :data:`False`. So long as + this is set to :data:`True`, then the code inside this scope + will not receive :exc:`~trio.Cancelled` exceptions from scopes + that are outside this scope. They can still receive + :exc:`~trio.Cancelled` exceptions from (1) this scope, or (2) + scopes inside this scope. You can modify this attribute:: + + with trio.CancelScope() as cancel_scope: + cancel_scope.shield = True + # This cannot be interrupted by any means short of + # killing the process: + await sleep(10) + + cancel_scope.shield = False + # Now this can be cancelled normally: + await sleep(10) + + Defaults to :data:`False`, though this can be overridden by the + ``shield=`` argument to the :class:`~trio.CancelScope` constructor. + """ return self._shield @shield.setter @@ -305,6 +346,11 @@ def _cancel_no_notify(self): @enable_ki_protection def cancel(self): + """Cancels this scope immediately. + + This method is idempotent, i.e., if the scope was already + cancelled then this method silently does nothing. + """ for task in self._cancel_no_notify(): task._attempt_delivery_of_any_pending_cancel() diff --git a/trio/_core/tests/test_run.py b/trio/_core/tests/test_run.py index 0c8a511e49..c1d2aa0b8f 100644 --- a/trio/_core/tests/test_run.py +++ b/trio/_core/tests/test_run.py @@ -892,17 +892,23 @@ async def sleep_until_cancelled(scope): assert "may not be entered while it is already active" in str( exc_info.value ) - async with _core.open_nursery() as nursery: - async def try_in_other_task(): - with pytest.raises(RuntimeError) as exc_info: - with scope: - pass # pragma: no cover - assert "while it is already active in another task" in str( - exc_info.value - ) + # Attempts to enter from two tasks simultaneously throw an error + async def enter_scope(): + with scope: + await sleep_forever() + + async with _core.open_nursery() as nursery: + nursery.start_soon(enter_scope) + await wait_all_tasks_blocked() - nursery.start_soon(try_in_other_task) + with pytest.raises(RuntimeError) as exc_info: + with scope: + pass # pragma: no cover + assert "while it is already active in another task" in str( + exc_info.value + ) + nursery.cancel_scope.cancel() async def test_timekeeping():