Merge pull request #349 from goodboy/prompt_on_ctrlc

Re-draw `pdbpp` prompt on `SIGINT`
goodboy · Jan 26, 2023 · de93c82 · de93c82
2 parents 588b7ca + 5b8a87d
commit de93c82
Show file tree

Hide file tree

Showing 6 changed files with 120 additions and 50 deletions.
diff --git a/nooz/349.trivial.rst 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.
diff --git a/setup.py b/setup.py
@@ -60,6 +60,9 @@
         # 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 @@
         # 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'],

diff --git 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

diff --git a/tractor/_debug.py b/tractor/_debug.py
@@ -20,9 +20,13 @@
 """
 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,
@@ -73,6 +77,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 +116,7 @@ class Lock:
     def shield_sigint(cls):
         cls._orig_sigint_handler = signal.signal(
             signal.SIGINT,
-            shield_sigint,
+            shield_sigint_handler,
         )
 
     @classmethod
@@ -146,6 +151,7 @@ def release(cls):
         finally:
             # restore original sigint handler
             cls.unshield_sigint()
+            cls.repl = None
 
 
 class TractorConfig(pdbpp.DefaultConfig):
@@ -184,6 +190,35 @@ def set_quit(self):
         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()
+
+    @cached_property
+    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
+        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..
+        shpath = os.getenv('SHELL', None)
+
+        if shpath:
+            if (
+                os.getenv('XONSH_LOGIN', default=False)
+                or 'xonsh' in shpath
+            ):
+                return 'xonsh'
+
+            return os.path.basename(shpath)
+
+        return None
+
 
 @acm
 async def _acquire_debug_lock_from_root_task(
@@ -388,6 +423,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 +471,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 +523,7 @@ async def _breakpoint(
                     wait_for_parent_stdin_hijack,
                     actor.uid,
                 )
+                Lock.repl = pdb
         except RuntimeError:
             Lock.release()
 
@@ -522,6 +562,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 +586,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 +606,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 +640,9 @@ def do_cancel():
                 )
                 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 +657,34 @@ def do_cancel():
     ):
         # 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 +700,10 @@ def do_cancel():
             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}`"
             )
@@ -668,14 +718,21 @@ def do_cancel():
         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.
 
-    # 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)
+        pdb_obj  # only when this actor has a REPL engaged
     ):
+        # 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
         # detected as written to the tty we redraw this part underneath
         # and erase the past draw of this same bit above?
@@ -689,14 +746,6 @@ def do_cancel():
         # 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 +869,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 (

diff --git 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

diff --git 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),