Skip to content

Commit

Permalink
Fixed timeout errors not propagating from nested scopes (agronholm#59)
Browse files Browse the repository at this point in the history
Check all parents for cancellation, not just the immediate one.

Fixes agronholm#58.
  • Loading branch information
smurfix authored and agronholm committed May 10, 2019
1 parent 8f33253 commit aa5b388
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 8 deletions.
30 changes: 26 additions & 4 deletions anyio/_backends/asyncio.py
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ async def __aexit__(self, exc_type, exc_val, exc_tb):
if all(isinstance(exc, CancelledError) for exc in exceptions):
if self._timeout_expired:
return True
elif self._parent_scope is None or not self._parent_scope.cancel_called:
elif not self._parent_cancelled():
# This scope was directly cancelled
return True

Expand All @@ -246,9 +246,32 @@ async def _cancel(self):
# Only deliver the cancellation if the task is already running
if task._coro.cr_await is not None:
task.cancel()
elif not cancel_scope.shield:
elif not cancel_scope._shielded_to(self):
await cancel_scope._cancel()

def _shielded_to(self, parent: Optional['CancelScope']) -> bool:
# Check whether this task or any parent up to (but not including) the "parent" argument is
# shielded
cancel_scope = self # type: Optional[CancelScope]
while cancel_scope is not None and cancel_scope is not parent:
if cancel_scope._shield:
return True
else:
cancel_scope = cancel_scope._parent_scope

return False

def _parent_cancelled(self) -> bool:
# Check whether any parent has been cancelled
cancel_scope = self._parent_scope
while cancel_scope is not None and not cancel_scope._shield:
if cancel_scope._cancel_called:
return True
else:
cancel_scope = cancel_scope._parent_scope

return False

async def cancel(self):
if self._cancel_called:
return
Expand Down Expand Up @@ -379,8 +402,7 @@ async def __aexit__(self, exc_type, exc_val, exc_tb):
await asyncio.wait(self.cancel_scope._tasks)

self._active = False
if (not self.cancel_scope._parent_scope
or not self.cancel_scope._parent_scope.cancel_called):
if not self.cancel_scope._parent_cancelled():
exceptions = [exc for exc in self._exceptions if not isinstance(exc, CancelledError)]
else:
exceptions = self._exceptions
Expand Down
30 changes: 26 additions & 4 deletions anyio/_backends/curio.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ async def __aexit__(self, exc_type, exc_val, exc_tb):
if all(isinstance(exc, CancelledError) for exc in exceptions):
if self._timeout_expired:
return True
elif self._parent_scope is None or not self._parent_scope.cancel_called:
elif not self._parent_cancelled():
# This scope was directly cancelled
return True

Expand All @@ -125,9 +125,32 @@ async def _cancel(self):
# Only deliver the cancellation if the task is already running
if task.coro.cr_await is not None:
await task.cancel(blocking=False)
elif not cancel_scope.shield:
elif not cancel_scope._shielded_to(self):
await cancel_scope._cancel()

def _shielded_to(self, parent: 'CancelScope') -> bool:
# Check whether this task or any parent up to (but not including) the "parent" argument is
# shielded
cancel_scope = self # type: Optional[CancelScope]
while cancel_scope is not None and cancel_scope is not parent:
if cancel_scope._shield:
return True
else:
cancel_scope = cancel_scope._parent_scope

return False

def _parent_cancelled(self) -> bool:
# Check whether any parent has been cancelled
cancel_scope = self._parent_scope
while cancel_scope is not None and not cancel_scope._shield:
if cancel_scope._cancel_called:
return True
else:
cancel_scope = cancel_scope._parent_scope

return False

async def cancel(self):
if self._cancel_called:
return
Expand Down Expand Up @@ -256,8 +279,7 @@ async def __aexit__(self, exc_type, exc_val, exc_tb):
await task.wait()

self._active = False
if (not self.cancel_scope._parent_scope
or not self.cancel_scope._parent_scope.cancel_called):
if not self.cancel_scope._parent_cancelled():
exceptions = [exc for exc in self._exceptions if not isinstance(exc, CancelledError)]
else:
exceptions = self._exceptions
Expand Down
1 change: 1 addition & 0 deletions docs/versionhistory.rst
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ This library adheres to `Semantic Versioning <http://semver.org/>`_.
**UNRELEASED**

- Fixed pathlib2_ compatibility with ``anyio.aopen()``
- Fixed timeouts not propagating from nested scopes on asyncio and curio (PR by Matthias Urlichs)

.. _pathlib2: https://pypi.org/project/pathlib2/

Expand Down
36 changes: 36 additions & 0 deletions tests/test_taskgroups.py
Original file line number Diff line number Diff line change
Expand Up @@ -430,3 +430,39 @@ async def test_catch_cancellation():
raise

assert finalizer_done


@pytest.mark.anyio
async def test_nested_fail_after():
async def killer(scope):
await wait_all_tasks_blocked()
await scope.cancel()

async with create_task_group() as tg:
async with open_cancel_scope() as scope:
async with open_cancel_scope():
await tg.spawn(killer, scope)
async with fail_after(1):
await sleep(2)
pytest.fail('Execution should not reach this point')

pytest.fail('Execution should not reach this point either')

pytest.fail('Execution should also not reach this point')

assert scope.cancel_called


@pytest.mark.anyio
async def test_nested_shield():
async def killer(scope):
await wait_all_tasks_blocked()
await scope.cancel()

with pytest.raises(TimeoutError):
async with create_task_group() as tg:
async with open_cancel_scope() as scope:
async with open_cancel_scope(shield=True):
await tg.spawn(killer, scope)
async with fail_after(0.2):
await sleep(2)

0 comments on commit aa5b388

Please sign in to comment.