From e51c2620e5fe30ec80208106945f14cac46c21c4 Mon Sep 17 00:00:00 2001 From: Tyler Goodlet Date: Fri, 11 Dec 2020 00:15:09 -0500 Subject: [PATCH 1/3] End the `pdb` SIGINT handling madness Turns out this is a lower level issue in terms of the stdlib's default `pdb.Pdb` settings and how they conflict with `trio`s cancellation and KBI handling. The details are hashed out more thoroughly in python-trio/trio#1155. Maybe we can get a fix in trio so things are solved under our feet :) --- tractor/__init__.py | 10 +++++++--- tractor/_actor.py | 4 ++-- tractor/_debug.py | 38 ++++++++++++++++++++------------------ tractor/_entry.py | 12 +++++++----- 4 files changed, 36 insertions(+), 28 deletions(-) diff --git a/tractor/__init__.py b/tractor/__init__.py index adaad387b..070bf72b5 100644 --- a/tractor/__init__.py +++ b/tractor/__init__.py @@ -117,9 +117,13 @@ async def _main( # Note that if the current actor is the arbiter it is desirable # for it to stay up indefinitely until a re-election process has # taken place - which is not implemented yet FYI). - return await _start_actor( - actor, main, host, port, arbiter_addr=arbiter_addr - ) + + try: + return await _start_actor( + actor, main, host, port, arbiter_addr=arbiter_addr + ) + finally: + logger.info("Root actor terminated") def run( diff --git a/tractor/_actor.py b/tractor/_actor.py index 6d223b088..03ff833f2 100644 --- a/tractor/_actor.py +++ b/tractor/_actor.py @@ -767,8 +767,8 @@ async def _async_main( finally: log.info("Root nursery complete") - # tear down all lifetime contexts - # api idea: ``tractor.open_context()`` + # tear down all lifetime contexts if not in guest mode + # XXX: should this just be in the entrypoint? log.warning("Closing all actor lifetime contexts") self._lifetime_stack.close() diff --git a/tractor/_debug.py b/tractor/_debug.py index 29c0430e4..bb73857f5 100644 --- a/tractor/_debug.py +++ b/tractor/_debug.py @@ -6,12 +6,10 @@ from functools import partial from contextlib import asynccontextmanager from typing import Awaitable, Tuple, Optional, Callable, AsyncIterator -# import signal from async_generator import aclosing import tractor import trio -from trio.testing import wait_all_tasks_blocked from .log import get_logger from . import _state @@ -132,19 +130,6 @@ async def _acquire_debug_lock(uid: Tuple[str, str]) -> AsyncIterator[None]: log.error(f"TTY lock released by {task_name}:{uid}") -def handler(signum, frame): - """Block SIGINT while in debug to avoid deadlocks with cancellation. - """ - print( - "tractor ignores SIGINT while in debug mode\n" - "If you have a special need for it please open an issue.\n" - ) - - -# don't allow those stdlib mofos to mess with sigint handler -pdbpp.pdb.Pdb.sigint_handler = handler - - # @contextmanager # def _disable_sigint(): # try: @@ -269,14 +254,29 @@ async def _bp(): log.debug("Entering the synchronous world of pdb") debug_func(actor) - # user code **must** await this! return _bp() +def _mk_pdb(): + # XXX: setting these flags on the pdb instance are absolutely + # critical to having ctrl-c work in the ``trio`` standard way! + # The stdlib's pdb supports entering the current sync frame + # on a SIGINT, with ``trio`` we pretty much never want this + # and we did we can handle it in the ``tractor`` task runtime. + + pdb = PdbwTeardown() + pdb.allow_kbdint = True + pdb.nosigint = True + + return pdb + + def _set_trace(actor): log.critical(f"\nAttaching pdb to actor: {actor.uid}\n") - PdbwTeardown().set_trace( + + pdb = _mk_pdb() + pdb.set_trace( # start 2 levels up in user code frame=sys._getframe().f_back.f_back, ) @@ -290,8 +290,10 @@ def _set_trace(actor): def _post_mortem(actor): log.critical(f"\nAttaching to pdb in crashed actor: {actor.uid}\n") + pdb = _mk_pdb() + # custom Pdb post-mortem entry - pdbpp.xpm(Pdb=PdbwTeardown) + pdbpp.xpm(Pdb=lambda: pdb) post_mortem = partial( diff --git a/tractor/_entry.py b/tractor/_entry.py index 9099fc022..a9b91e413 100644 --- a/tractor/_entry.py +++ b/tractor/_entry.py @@ -7,7 +7,6 @@ import trio # type: ignore -from ._actor import Actor from .log import get_console_log, get_logger from . import _state @@ -16,7 +15,7 @@ def _mp_main( - actor: 'Actor', + actor: 'Actor', # noqa accept_addr: Tuple[str, int], forkserver_info: Tuple[Any, Any, Any, Any, Any], start_method: str, @@ -49,11 +48,13 @@ def _mp_main( trio.run(trio_main) except KeyboardInterrupt: pass # handle it the same way trio does? - log.info(f"Actor {actor.uid} terminated") + + finally: + log.info(f"Actor {actor.uid} terminated") def _trio_main( - actor: 'Actor', + actor: 'Actor', # noqa parent_addr: Tuple[str, int] = None ) -> None: """Entry point for a `trio_run_in_process` subactor. @@ -86,4 +87,5 @@ def _trio_main( except KeyboardInterrupt: log.warning(f"Actor {actor.uid} received KBI") - log.info(f"Actor {actor.uid} terminated") + finally: + log.info(f"Actor {actor.uid} terminated") From d497078eb740984a0f63de9c7e35e5da8bf2b25d Mon Sep 17 00:00:00 2001 From: Tyler Goodlet Date: Thu, 10 Dec 2020 14:07:36 -0500 Subject: [PATCH 2/3] Appease 3.8 mypy --- tractor/_actor.py | 9 +++++---- tractor/_entry.py | 7 ++++--- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/tractor/_actor.py b/tractor/_actor.py index 03ff833f2..02a010c62 100644 --- a/tractor/_actor.py +++ b/tractor/_actor.py @@ -821,7 +821,7 @@ async def _serve_forever( self._server_down = trio.Event() try: async with trio.open_nursery() as server_n: - listeners: List[trio.abc.Listener] = await server_n.start( + l: List[trio.abc.Listener] = await server_n.start( partial( trio.serve_tcp, self._stream_handler, @@ -832,9 +832,10 @@ async def _serve_forever( host=accept_host, ) ) - log.debug("Started tcp server(s) on" # type: ignore - f" {[l.socket for l in listeners]}") - self._listeners.extend(listeners) + log.debug( + "Started tcp server(s) on" + f" {[getattr(l, 'socket', 'unknown socket') for l in l]}") + self._listeners.extend(l) task_status.started(server_n) finally: # signal the server is down since nursery above terminated diff --git a/tractor/_entry.py b/tractor/_entry.py index a9b91e413..0faf486cb 100644 --- a/tractor/_entry.py +++ b/tractor/_entry.py @@ -15,7 +15,7 @@ def _mp_main( - actor: 'Actor', # noqa + actor: 'Actor', # type: ignore accept_addr: Tuple[str, int], forkserver_info: Tuple[Any, Any, Any, Any, Any], start_method: str, @@ -54,8 +54,9 @@ def _mp_main( def _trio_main( - actor: 'Actor', # noqa - parent_addr: Tuple[str, int] = None + actor: 'Actor', # type: ignore + *, + parent_addr: Tuple[str, int] = None, ) -> None: """Entry point for a `trio_run_in_process` subactor. """ From 0118589875b678ad68b1fe8c6a3d6e66255a85e3 Mon Sep 17 00:00:00 2001 From: Tyler Goodlet Date: Sat, 12 Dec 2020 13:29:22 -0500 Subject: [PATCH 3/3] Add race case handling for mp backend --- .../root_cancelled_but_child_is_in_tty_lock.py | 3 ++- tests/test_debugger.py | 12 ++++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/examples/debugging/root_cancelled_but_child_is_in_tty_lock.py b/examples/debugging/root_cancelled_but_child_is_in_tty_lock.py index 04ec7672c..797de9c17 100644 --- a/examples/debugging/root_cancelled_but_child_is_in_tty_lock.py +++ b/examples/debugging/root_cancelled_but_child_is_in_tty_lock.py @@ -39,7 +39,8 @@ async def main(): portal = await n.run_in_actor('spawner0', spawn_until, depth=0) portal1 = await n.run_in_actor('spawner1', spawn_until, depth=1) - # nursery cancellation should be triggered due to propagated error + # nursery cancellation should be triggered due to propagated + # error from child. await portal.result() await portal1.result() diff --git a/tests/test_debugger.py b/tests/test_debugger.py index fc94b6fdb..53c3c84be 100644 --- a/tests/test_debugger.py +++ b/tests/test_debugger.py @@ -365,7 +365,7 @@ def test_multi_nested_subactors_error_through_nurseries(spawn): assert "NameError" in before -def test_root_nursery_cancels_before_child_releases_tty_lock(spawn): +def test_root_nursery_cancels_before_child_releases_tty_lock(spawn, start_method): """Test that when the root sends a cancel message before a nested child has unblocked (which can happen when it has the tty lock and is engaged in pdb) it is indeed cancelled after exiting the debugger. @@ -380,7 +380,15 @@ def test_root_nursery_cancels_before_child_releases_tty_lock(spawn): child.sendline('c') for _ in range(4): - child.expect(r"\(Pdb\+\+\)") + try: + child.expect(r"\(Pdb\+\+\)") + except TimeoutError: + if start_method == 'mp': + # appears to be some little races that might result in the + # last couple acts tearing down early + break + else: + raise before = str(child.before.decode()) assert "NameError: name 'doggypants' is not defined" in before