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

End the pdb SIGINT handling madness #170

Merged
merged 3 commits into from
Dec 12, 2020
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
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
12 changes: 10 additions & 2 deletions tests/test_debugger.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down
10 changes: 7 additions & 3 deletions tractor/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are probably superfluous (and maybe should be .debug()) but figured can't hurt atm. Likely we'll throttle back logging noise before first .dev0.



def run(
Expand Down
13 changes: 7 additions & 6 deletions tractor/_actor.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down Expand Up @@ -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,
Expand All @@ -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]}")
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just mypy stuff since technically .socket isn't discovered through the type system easily.

self._listeners.extend(l)
task_status.started(server_n)
finally:
# signal the server is down since nursery above terminated
Expand Down
38 changes: 20 additions & 18 deletions tractor/_debug.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out we can just keep trio standard KBI handling as long as we prevent pdb.Pdb from overriding it!

"""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:
Expand Down Expand Up @@ -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.
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be and if we did..


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,
)
Expand All @@ -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(
Expand Down
15 changes: 9 additions & 6 deletions tractor/_entry.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

import trio # type: ignore

from ._actor import Actor
from .log import get_console_log, get_logger
from . import _state

Expand All @@ -16,7 +15,7 @@


def _mp_main(
actor: 'Actor',
actor: 'Actor', # type: ignore
accept_addr: Tuple[str, int],
forkserver_info: Tuple[Any, Any, Any, Any, Any],
start_method: str,
Expand Down Expand Up @@ -49,12 +48,15 @@ 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',
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.
"""
Expand Down Expand Up @@ -86,4 +88,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")