From 5ed62c5c54fb4c4c58773c66a959a4714b72fae1 Mon Sep 17 00:00:00 2001 From: Tyler Goodlet Date: Wed, 25 Jan 2023 18:30:18 -0500 Subject: [PATCH 1/8] Add note about intermediary-actor in debug issue --- tractor/_spawn.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tractor/_spawn.py b/tractor/_spawn.py index 7380ea0..6c0ce5b 100644 --- a/tractor/_spawn.py +++ b/tractor/_spawn.py @@ -457,6 +457,13 @@ async def trio_proc( await proc.wait() if is_root_process(): + # TODO: solve the following issue where we need + # to do a similar wait like this but in an + # "intermediary" parent actor that itself isn't + # in debug but has a child that is, and we need + # to hold off on relaying SIGINT until that child + # is complete. + # https://github.com/goodboy/tractor/issues/320 await maybe_wait_for_debugger( child_in_debug=_runtime_vars.get( '_debug_mode', False), From fca2e7c10e85346a3cd3358bb1b72d9ecacac4ad Mon Sep 17 00:00:00 2001 From: Tyler Goodlet Date: Wed, 25 Jan 2023 19:13:34 -0500 Subject: [PATCH 2/8] Simplify closed abruptly log msg --- tractor/_runtime.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tractor/_runtime.py b/tractor/_runtime.py index 12b1f82..e826258 100644 --- a/tractor/_runtime.py +++ b/tractor/_runtime.py @@ -1599,7 +1599,10 @@ async def process_messages( # handshake for them (yet) and instead we simply bail out of # the message loop and expect the teardown sequence to clean # up. - log.runtime(f'channel from {chan.uid} closed abruptly:\n{chan}') + log.runtime( + f'channel from {chan.uid} closed abruptly:\n' + f'-> {chan.raddr}\n' + ) # transport **was** disconnected return True From dba811855361af3d5a29f7c49e6b7ed8526c650d Mon Sep 17 00:00:00 2001 From: Tyler Goodlet Date: Thu, 26 Jan 2023 11:55:32 -0500 Subject: [PATCH 3/8] Always attempt prompt redraw on ctl-c in REPL The stdlib has all sorts of muckery with ignoring SIGINT in the `Pdb._cmdloop()` but here we just override all that since we don't trust their decisions about cancellation handling whatsoever. Adds a `Lock.repl: MultiActorPdb` attr which is set by any task which acquires root TTY lock indicating (via actor global state) that the current actor is using the debugger REPL and can be expected to re-draw the prompt on SIGINT. Further we mask out log messages from any actor who also has the `shield_sigint_handler()` enabled to avoid logging noise when debugging. --- tractor/_debug.py | 85 ++++++++++++++++++++++++++++++----------------- 1 file changed, 54 insertions(+), 31 deletions(-) diff --git a/tractor/_debug.py b/tractor/_debug.py index af1bf55..bfde43b 100644 --- a/tractor/_debug.py +++ b/tractor/_debug.py @@ -73,6 +73,7 @@ class Lock: Mostly to avoid a lot of ``global`` declarations for now XD. ''' + repl: MultiActorPdb | None = None # placeholder for function to set a ``trio.Event`` on debugger exit # pdb_release_hook: Optional[Callable] = None @@ -111,7 +112,7 @@ class Lock: def shield_sigint(cls): cls._orig_sigint_handler = signal.signal( signal.SIGINT, - shield_sigint, + shield_sigint_handler, ) @classmethod @@ -146,6 +147,7 @@ class Lock: finally: # restore original sigint handler cls.unshield_sigint() + cls.repl = None class TractorConfig(pdbpp.DefaultConfig): @@ -184,6 +186,12 @@ class MultiActorPdb(pdbpp.Pdb): finally: Lock.release() + # XXX NOTE: we only override this because apparently the stdlib pdb + # bois likes to touch the SIGINT handler as much as i like to touch + # my d$%&. + def _cmdloop(self): + self.cmdloop() + @acm async def _acquire_debug_lock_from_root_task( @@ -388,6 +396,7 @@ async def wait_for_parent_stdin_hijack( except ContextCancelled: log.warning('Root actor cancelled debug lock') + raise finally: Lock.local_task_in_debug = None @@ -435,7 +444,10 @@ async def _breakpoint( # with trio.CancelScope(shield=shield): # await trio.lowlevel.checkpoint() - if not Lock.local_pdb_complete or Lock.local_pdb_complete.is_set(): + if ( + not Lock.local_pdb_complete + or Lock.local_pdb_complete.is_set() + ): Lock.local_pdb_complete = trio.Event() # TODO: need a more robust check for the "root" actor @@ -484,6 +496,7 @@ async def _breakpoint( wait_for_parent_stdin_hijack, actor.uid, ) + Lock.repl = pdb except RuntimeError: Lock.release() @@ -522,6 +535,7 @@ async def _breakpoint( Lock.global_actor_in_debug = actor.uid Lock.local_task_in_debug = task_name + Lock.repl = pdb try: # block here one (at the appropriate frame *up*) where @@ -545,10 +559,10 @@ async def _breakpoint( # # signal.signal = pdbpp.hideframe(signal.signal) -def shield_sigint( +def shield_sigint_handler( signum: int, frame: 'frame', # type: ignore # noqa - pdb_obj: Optional[MultiActorPdb] = None, + # pdb_obj: Optional[MultiActorPdb] = None, *args, ) -> None: @@ -565,6 +579,7 @@ def shield_sigint( uid_in_debug = Lock.global_actor_in_debug actor = tractor.current_actor() + # print(f'{actor.uid} in HANDLER with ') def do_cancel(): # If we haven't tried to cancel the runtime then do that instead @@ -598,6 +613,9 @@ def shield_sigint( ) return do_cancel() + # only set in the actor actually running the REPL + pdb_obj = Lock.repl + # root actor branch that reports whether or not a child # has locked debugger. if ( @@ -612,32 +630,34 @@ def shield_sigint( ): # we are root and some actor is in debug mode # if uid_in_debug is not None: - name = uid_in_debug[0] - if name != 'root': - log.pdb( - f"Ignoring SIGINT while child in debug mode: `{uid_in_debug}`" - ) - else: - log.pdb( - "Ignoring SIGINT while in debug mode" - ) + if pdb_obj: + name = uid_in_debug[0] + if name != 'root': + log.pdb( + f"Ignoring SIGINT, child in debug mode: `{uid_in_debug}`" + ) + + else: + log.pdb( + "Ignoring SIGINT while in debug mode" + ) elif ( is_root_process() ): - log.pdb( - "Ignoring SIGINT since debug mode is enabled" - ) + if pdb_obj: + log.pdb( + "Ignoring SIGINT since debug mode is enabled" + ) - # revert back to ``trio`` handler asap! - Lock.unshield_sigint() if ( Lock._root_local_task_cs_in_debug and not Lock._root_local_task_cs_in_debug.cancel_called ): Lock._root_local_task_cs_in_debug.cancel() - # raise KeyboardInterrupt + # revert back to ``trio`` handler asap! + Lock.unshield_sigint() # child actor that has locked the debugger elif not is_root_process(): @@ -653,7 +673,10 @@ def shield_sigint( return do_cancel() task = Lock.local_task_in_debug - if task: + if ( + task + and pdb_obj + ): log.pdb( f"Ignoring SIGINT while task in debug mode: `{task}`" ) @@ -671,11 +694,16 @@ def shield_sigint( # it lookks to be that the last command that was run (eg. ll) # will be repeated by default. - # TODO: maybe redraw/print last REPL output to console + # maybe redraw/print last REPL output to console since + # we want to alert the user that more input is expect since + # nothing has been done dur to ignoring sigint. if ( pdb_obj - and sys.version_info <= (3, 10) ): + # redraw the prompt ONLY in the actor that has the REPL running. + pdb_obj.stdout.write(pdb_obj.prompt) + pdb_obj.stdout.flush() + # TODO: make this work like sticky mode where if there is output # detected as written to the tty we redraw this part underneath # and erase the past draw of this same bit above? @@ -689,14 +717,6 @@ def shield_sigint( # XXX: lol, see ``pdbpp`` issue: # https://github.com/pdbpp/pdbpp/issues/496 - # TODO: pretty sure this is what we should expect to have to run - # in total but for now we're just going to wait until `pdbpp` - # figures out it's own stuff on 3.10 (and maybe we'll help). - # pdb_obj.do_longlist(None) - - # XXX: we were doing this but it shouldn't be required.. - print(pdb_obj.prompt, end='', flush=True) - def _set_trace( actor: Optional[tractor.Actor] = None, @@ -820,7 +840,10 @@ async def maybe_wait_for_debugger( ) -> None: - if not debug_mode() and not child_in_debug: + if ( + not debug_mode() + and not child_in_debug + ): return if ( From 6d124db7c96a7c2e2a180a93e46d626fc701c9fa Mon Sep 17 00:00:00 2001 From: Tyler Goodlet Date: Thu, 26 Jan 2023 12:02:30 -0500 Subject: [PATCH 4/8] Never run ctlc-with-intermediary-actor cases locally either --- tests/test_debugger.py | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/tests/test_debugger.py b/tests/test_debugger.py index dd5ab46..0793df5 100644 --- a/tests/test_debugger.py +++ b/tests/test_debugger.py @@ -165,17 +165,16 @@ def ctlc( # be 3.10+ mega-asap. pytest.skip('Py3.9 and `pdbpp` son no bueno..') - if ci_env: - node = request.node - markers = node.own_markers - for mark in markers: - if mark.name == 'has_nested_actors': - pytest.skip( - f'Test for {node} uses nested actors and fails in CI\n' - f'The test seems to run fine locally but until we solve' - 'this issue this CI test will be xfail:\n' - 'https://github.com/goodboy/tractor/issues/320' - ) + node = request.node + markers = node.own_markers + for mark in markers: + if mark.name == 'has_nested_actors': + pytest.skip( + f'Test {node} has nested actors and fails with Ctrl-C.\n' + f'The test can sometimes run fine locally but until' + ' we solve' 'this issue this CI test will be xfail:\n' + 'https://github.com/goodboy/tractor/issues/320' + ) if use_ctlc: # XXX: disable pygments highlighting for auto-tests From 2e278ceb7450cc21182478366fef86a454ef4540 Mon Sep 17 00:00:00 2001 From: Tyler Goodlet Date: Thu, 26 Jan 2023 15:26:43 -0500 Subject: [PATCH 5/8] Add a super hacky check for `xonsh`, smh.. --- tractor/_debug.py | 32 +++++++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/tractor/_debug.py b/tractor/_debug.py index bfde43b..be0e8de 100644 --- a/tractor/_debug.py +++ b/tractor/_debug.py @@ -20,9 +20,13 @@ Multi-core debugging for da peeps! """ from __future__ import annotations import bdb +import os import sys import signal -from functools import partial +from functools import ( + partial, + cached_property, +) from contextlib import asynccontextmanager as acm from typing import ( Any, @@ -192,6 +196,22 @@ class MultiActorPdb(pdbpp.Pdb): def _cmdloop(self): self.cmdloop() + @cached_property + def shname(self) -> str: + ''' + Attempt to return the login shell name with a special check for + the infamous `xonsh` since it seems to have some issues much + different from std shells when it comes to flushing the prompt? + + ''' + # SUPER HACKY and only really works if `xonsh` is not used + # before spawning further sub-shells.. + xonsh_is_login_sh: bool = os.getenv('XONSH_LOGIN', default=False) + if xonsh_is_login_sh: + return 'xonsh' + + return os.path.basename(os.getenv('SHELL')) + @acm async def _acquire_debug_lock_from_root_task( @@ -691,17 +711,19 @@ def shield_sigint_handler( raise KeyboardInterrupt # NOTE: currently (at least on ``fancycompleter`` 0.9.2) - # it lookks to be that the last command that was run (eg. ll) + # it looks to be that the last command that was run (eg. ll) # will be repeated by default. # maybe redraw/print last REPL output to console since # we want to alert the user that more input is expect since # nothing has been done dur to ignoring sigint. if ( - pdb_obj + pdb_obj # only when this actor has a REPL engaged ): - # redraw the prompt ONLY in the actor that has the REPL running. - pdb_obj.stdout.write(pdb_obj.prompt) + # XXX: yah, mega hack, but how else do we catch this madness XD + if pdb_obj.shname == 'xonsh': + pdb_obj.stdout.write(pdb_obj.prompt) + pdb_obj.stdout.flush() # TODO: make this work like sticky mode where if there is output From 965cd406a2e52077df692a76430ba41931eee56f Mon Sep 17 00:00:00 2001 From: Tyler Goodlet Date: Thu, 26 Jan 2023 15:27:55 -0500 Subject: [PATCH 6/8] Use std `pdbpp` release --- setup.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/setup.py b/setup.py index 0eabc5b..88d6612 100755 --- a/setup.py +++ b/setup.py @@ -60,6 +60,9 @@ setup( # serialization 'msgspec', + # debug mode REPL + 'pdbpp', + # pip ref docs on these specs: # https://pip.pypa.io/en/stable/reference/requirement-specifiers/#examples # and pep: @@ -70,10 +73,6 @@ setup( # https://github.com/pdbpp/fancycompleter/issues/37 'pyreadline3 ; platform_system == "Windows"', - # 3.10 has an outstanding unreleased issue and `pdbpp` itself - # pins to patched forks of its own dependencies as well..and - # we need a specific patch on master atm. - 'pdbpp @ git+https://github.com/pdbpp/pdbpp@76c4be5#egg=pdbpp ; python_version > "3.9"', # noqa: E501 ], tests_require=['pytest'], From 9e5c8ce6f6642d8715088cf029fe68b49995279d Mon Sep 17 00:00:00 2001 From: Tyler Goodlet Date: Thu, 26 Jan 2023 15:39:03 -0500 Subject: [PATCH 7/8] Add nooz file --- nooz/349.trivial.rst | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 nooz/349.trivial.rst diff --git a/nooz/349.trivial.rst b/nooz/349.trivial.rst new file mode 100644 index 0000000..6961886 --- /dev/null +++ b/nooz/349.trivial.rst @@ -0,0 +1,10 @@ +Always redraw the `pdbpp` prompt on `SIGINT` during REPL use. + +There was recent changes todo with Python 3.10 that required us to pin +to a specific commit in `pdbpp` which have recently been fixed minus +this last issue with `SIGINT` shielding: not clobbering or not +showing the `(Pdb++)` prompt on ctlr-c by the user. This repairs all +that by firstly removing the standard KBI intercepting of the std lib's +`pdb.Pdb._cmdloop()` as well as ensuring that only the actor with REPL +control ever reports `SIGINT` handler log msgs and prompt redraws. With +this we move back to using pypi `pdbpp` release. From 5b8a87d0f61506d3a9d93dd4466f7b965b539599 Mon Sep 17 00:00:00 2001 From: Tyler Goodlet Date: Thu, 26 Jan 2023 15:48:15 -0500 Subject: [PATCH 8/8] Slightly better `xonsh` check hack, fix typing --- tractor/_debug.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/tractor/_debug.py b/tractor/_debug.py index be0e8de..47a9a88 100644 --- a/tractor/_debug.py +++ b/tractor/_debug.py @@ -197,7 +197,7 @@ class MultiActorPdb(pdbpp.Pdb): self.cmdloop() @cached_property - def shname(self) -> str: + def shname(self) -> str | None: ''' Attempt to return the login shell name with a special check for the infamous `xonsh` since it seems to have some issues much @@ -206,11 +206,18 @@ class MultiActorPdb(pdbpp.Pdb): ''' # SUPER HACKY and only really works if `xonsh` is not used # before spawning further sub-shells.. - xonsh_is_login_sh: bool = os.getenv('XONSH_LOGIN', default=False) - if xonsh_is_login_sh: - return 'xonsh' + shpath = os.getenv('SHELL', None) - return os.path.basename(os.getenv('SHELL')) + if shpath: + if ( + os.getenv('XONSH_LOGIN', default=False) + or 'xonsh' in shpath + ): + return 'xonsh' + + return os.path.basename(shpath) + + return None @acm