From f60321a35ae8ff28c853e0da34c91f36ad195206 Mon Sep 17 00:00:00 2001 From: Tyler Goodlet Date: Wed, 14 Oct 2020 13:46:05 -0400 Subject: [PATCH 1/5] Always cancel service nursery last The channel server should be torn down *before* the rpc task/service nursery. Do this explicitly even in the root's main task to avoid a strange hang I found in the pubsub tests. Start dropping the `warnings.warn()` usage. --- tractor/_actor.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/tractor/_actor.py b/tractor/_actor.py index f4b9795..6010a56 100644 --- a/tractor/_actor.py +++ b/tractor/_actor.py @@ -150,7 +150,7 @@ async def _invoke( except KeyError: # If we're cancelled before the task returns then the # cancel scope will not have been inserted yet - log.warn( + log.warning( f"Task {func} likely errored or cancelled before it started") finally: if not actor._rpc_tasks: @@ -520,7 +520,7 @@ class Actor: # deadlock and other weird behaviour) if func != self.cancel: if isinstance(cs, Exception): - log.warn(f"Task for RPC func {func} failed with" + log.warning(f"Task for RPC func {func} failed with" f"{cs}") else: # mark that we have ongoing rpc tasks @@ -547,7 +547,7 @@ class Actor: log.debug( f"{chan} for {chan.uid} disconnected, cancelling tasks" ) - self.cancel_rpc_tasks(chan) + await self.cancel_rpc_tasks(chan) except trio.ClosedResourceError: log.error(f"{chan} form {chan.uid} broke") @@ -757,7 +757,7 @@ class Actor: # tear down all lifetime contexts # api idea: ``tractor.open_context()`` - log.warn("Closing all actor lifetime contexts") + log.warning("Closing all actor lifetime contexts") self._lifetime_stack.close() # Unregister actor from the arbiter @@ -855,14 +855,14 @@ class Actor: # kill all ongoing tasks await self.cancel_rpc_tasks() - # cancel all rpc tasks permanently - if self._service_n: - self._service_n.cancel_scope.cancel() - # stop channel server self.cancel_server() await self._server_down.wait() + # cancel all rpc tasks permanently + if self._service_n: + self._service_n.cancel_scope.cancel() + log.warning(f"{self.uid} was sucessfullly cancelled") self._cancel_complete.set() return True @@ -1095,6 +1095,7 @@ async def _start_actor( # XXX: the actor is cancelled when this context is complete # given that there are no more active peer channels connected actor.cancel_server() + actor._service_n.cancel_scope.cancel() # unset module state _state._current_actor = None From 02a9cac55709226460f26077692632e8127bb58c Mon Sep 17 00:00:00 2001 From: Tyler Goodlet Date: Wed, 14 Oct 2020 13:48:14 -0400 Subject: [PATCH 2/5] Drop remaining warn()s --- tractor/_forkserver_override.py | 4 ++-- tractor/_trionics.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tractor/_forkserver_override.py b/tractor/_forkserver_override.py index 07933dd..c515510 100644 --- a/tractor/_forkserver_override.py +++ b/tractor/_forkserver_override.py @@ -234,8 +234,8 @@ def main(listener_fd, alive_r, preload, main_path=None, sys_path=None): os.close(child_w) else: # This shouldn't happen really - warnings.warn('forkserver: waitpid returned ' - 'unexpected pid %d' % pid) + warnings.warning('forkserver: waitpid returned ' + 'unexpected pid %d' % pid) if listener in rfds: # Incoming fork request diff --git a/tractor/_trionics.py b/tractor/_trionics.py index aecf16e..203a9dc 100644 --- a/tractor/_trionics.py +++ b/tractor/_trionics.py @@ -275,7 +275,7 @@ async def open_nursery() -> typing.AsyncGenerator[ActorNursery, None]: # ria_nursery scope end # XXX: do we need a `trio.Cancelled` catch here as well? - except (Exception, trio.MultiError) as err: + except (Exception, trio.MultiError, trio.Cancelled) as err: # If actor-local error was raised while waiting on # ".run_in_actor()" actors then we also want to cancel all # remaining sub-actors (due to our lone strategy: From 676cdafa8f6e5950db653f42d2c295887adf9752 Mon Sep 17 00:00:00 2001 From: Tyler Goodlet Date: Wed, 14 Oct 2020 13:48:41 -0400 Subject: [PATCH 3/5] Try out sync sleep on windows again --- tests/test_cancellation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_cancellation.py b/tests/test_cancellation.py index 9e3c9f9..8cd760d 100644 --- a/tests/test_cancellation.py +++ b/tests/test_cancellation.py @@ -426,7 +426,7 @@ async def spawn(): portal = await tn.run_in_actor('sleeper', spin_for) -@no_windows +# @no_windows def test_cancel_while_childs_child_in_sync_sleep( loglevel, start_method, From 3b8684f655e328bf0385497227a0c0bd2d627bed Mon Sep 17 00:00:00 2001 From: Tyler Goodlet Date: Wed, 14 Oct 2020 13:59:57 -0400 Subject: [PATCH 4/5] Always call `Actor.cancel()` at end of root's main task It's simpler and the only real logical difference is logging messages. This should also give us an overall consistent tear down sequence. --- tractor/_actor.py | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/tractor/_actor.py b/tractor/_actor.py index 6010a56..178e12a 100644 --- a/tractor/_actor.py +++ b/tractor/_actor.py @@ -1083,19 +1083,12 @@ async def _start_actor( try: result = await main() except (Exception, trio.MultiError) as err: - try: - log.exception("Actor crashed:") - await _debug._maybe_enter_pm(err) + log.exception("Actor crashed:") + await _debug._maybe_enter_pm(err) - raise - - finally: - await actor.cancel() - - # XXX: the actor is cancelled when this context is complete - # given that there are no more active peer channels connected - actor.cancel_server() - actor._service_n.cancel_scope.cancel() + raise + finally: + await actor.cancel() # unset module state _state._current_actor = None From bd34140a5f54c91834710fdbc7d2a4b374824857 Mon Sep 17 00:00:00 2001 From: Tyler Goodlet Date: Wed, 14 Oct 2020 14:06:04 -0400 Subject: [PATCH 5/5] Revert "Try out sync sleep on windows again" Nope, still doesn't work... This reverts commit 676cdafa8f6e5950db653f42d2c295887adf9752. --- tests/test_cancellation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_cancellation.py b/tests/test_cancellation.py index 8cd760d..9e3c9f9 100644 --- a/tests/test_cancellation.py +++ b/tests/test_cancellation.py @@ -426,7 +426,7 @@ async def spawn(): portal = await tn.run_in_actor('sleeper', spin_for) -# @no_windows +@no_windows def test_cancel_while_childs_child_in_sync_sleep( loglevel, start_method,