Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-draw pdbpp prompt on SIGINT #349

Merged
merged 8 commits into from
Jan 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions nooz/349.trivial.rst
Original file line number Diff line number Diff line change
@@ -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.
7 changes: 3 additions & 4 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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'],
Expand Down
21 changes: 10 additions & 11 deletions tests/test_debugger.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
120 changes: 86 additions & 34 deletions tractor/_debug.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -111,7 +116,7 @@ class Lock:
def shield_sigint(cls):
cls._orig_sigint_handler = signal.signal(
signal.SIGINT,
shield_sigint,
shield_sigint_handler,
)

@classmethod
Expand Down Expand Up @@ -146,6 +151,7 @@ def release(cls):
finally:
# restore original sigint handler
cls.unshield_sigint()
cls.repl = None


class TractorConfig(pdbpp.DefaultConfig):
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -484,6 +523,7 @@ async def _breakpoint(
wait_for_parent_stdin_hijack,
actor.uid,
)
Lock.repl = pdb
except RuntimeError:
Lock.release()

Expand Down Expand Up @@ -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
Expand All @@ -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:
Expand All @@ -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
Expand Down Expand Up @@ -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 (
Expand All @@ -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():
Expand All @@ -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}`"
)
Expand All @@ -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?
Expand All @@ -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,
Expand Down Expand Up @@ -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 (
Expand Down
5 changes: 4 additions & 1 deletion tractor/_runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions tractor/_spawn.py
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down