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

Ignore SIGINT when in a debugger REPL #165

Merged
merged 100 commits into from
Aug 3, 2022
Merged

Ignore SIGINT when in a debugger REPL #165

merged 100 commits into from
Aug 3, 2022

Conversation

goodboy
Copy link
Owner

@goodboy goodboy commented Nov 16, 2020

Some fixes for handling SIGINT / control-c when running in guest debug mode and dealing with pdb internals that swap out the sigint handler from under our feet.

This resolves an issue when running the Qt loop in guest mode where control-c after a tractor.breakpoint() was resulting in a hang. There were 2 problems:
- our handler was borked (bad type signature)
- if we receive SIGINT in the root process we should always tear down the actor tree

There's been a long-standing bug to do with the root actor getting stuck hanging for the tty lock to release after a child locks it then becomes uncontactable over IPC.

Part of that was due to:

  • root and/or children accepting SIGINT (normally by accident since why would you want ctrl-c to work other then exiting the pdb repl while in the middle of it) while in the debugger which can trigger cancellation that will clobber the tty state
  • the root process always handling SIGINT when a child is in debug -> likely you want to wait until the child is done (especially if it's ignoring SIGINT while debugging) before handling it in the root and cancelling any actor nurseries..

still ToFix:
deferred to #320


still ToDo: deferred to #320


Next up:

  • ctl-c inside debugger REPL tests (for those that work)

    • single depth tree actor and root-only level tests:
    tests/test_debugger.py::test_root_actor_bp_forever[ctl-c=True-trio]
    tests/test_debugger.py::test_subactor_error[ctl-c=True-trio]
    tests/test_debugger.py::test_subactor_breakpoint[ctl-c=True-trio]
    tests/test_debugger.py::test_multi_subactors[ctl-c=True-trio]
    tests/test_debugger.py::test_multi_daemon_subactors[ctl-c=True-trio]
    tests/test_debugger.py::test_multi_subactors_root_errors[ctl-c=True-trio]
    tests/test_debugger.py::test_root_nursery_cancels_before_child_releases_tty_lock[ctl-c=True-trio]
    tests/test_debugger.py::test_root_cancels_child_context_during_startup[ctl-c=True-trio]
    tests/test_debugger.py::test_different_debug_mode_per_actor[ctl-c=True-trio]
    
  • follow up issue (set) for what we need to solve in terms of the much harder cases (Handling >= 1 depth (nested) actor trees in the debugger #320)

    • should we write expected to fail tests for these? skipped em instead bc xfail wasn't workin

@goodboy
Copy link
Owner Author

goodboy commented Dec 12, 2020

Pretty sure this get's defunct-ed by #170 but leaving up temporarily in case something here is needed for the new asyncio branch (which still has this history iirc).

Nope was still catching this during messy teardowns.

@goodboy goodboy force-pushed the signint_saviour branch 3 times, most recently from d86037a to f6f9b29 Compare August 3, 2021 13:09
@goodboy goodboy changed the title Signint saviour Ignore SIGINT when in a debugger REPL Dec 12, 2021
@goodboy
Copy link
Owner Author

goodboy commented Feb 9, 2022

Ok found an odd one while testing on 3.10. Basically if you don't have pdbpp master you'll end up with this bt on a traceback inside a @tractor.context block where you call .breakpoint() and then hit a remote error:

    Traceback (most recent call last):
    File ".../tractor/_debug.py", line 729, in _post_mortem
      for _ in range(100):
    File ".../tractor/310/lib/python3.10/site-packages/pdb.py", line 1227, in xpm
      post_mortem(info[2], Pdb)
    File "...tractor/310/lib/python3.10/site-packages/pdb.py", line 1175, in post_mortem
      p.interaction(None, t)
    File "...tractor/310/lib/python3.10/site-packages/pdb.py", line 216, in interaction
      ret = self.setup(frame, traceback)
    File "...tractor/310/lib/python3.10/site-packages/pdb.py", line 259, in setup
      ret = super(Pdb, self).setup(frame, tb)
    File "/usr/lib/python3.10/pdb.py", line 217, in setup
      self.curframe = self.stack[self.curindex][0]
    IndexError: list index out of range

Pinning to pdbpp's b757794857 seems to solve this 😎.
We probably should report issue that we're waiting on a release with this.

@goodboy
Copy link
Owner Author

goodboy commented Jun 26, 2022

heyooo CI passss...

I guess we're getting close?

Next up:

  • ctl-c inside debugger REPL tests (for those that work)
  • follow up issue (set) for what we need to solve in terms of the much harder cases

IMO this PR has been delayed way more then long enough and we might as well push out another alpha to get this in as well as #293

@goodboy
Copy link
Owner Author

goodboy commented Jun 28, 2022

Bleh, looks like pdbpp master and py3.9 aren't playing well..

@goodboy goodboy force-pushed the signint_saviour branch 2 times, most recently from 0acfa79 to 16cf07f Compare July 11, 2022 18:18
@goodboy
Copy link
Owner Author

goodboy commented Jul 11, 2022

Somehow this run passed: https://github.com/goodboy/tractor/runs/7063206690?check_suite_focus=true

But after that it doesn't?
I'm pretty lost on this with these weird timeouts now..

@goodboy goodboy mentioned this pull request Jul 11, 2022
@goodboy
Copy link
Owner Author

goodboy commented Jul 11, 2022

Lol, ok then.

@goodboy
Copy link
Owner Author

goodboy commented Aug 2, 2022

Squashed all the squashables, just needs nooz and I think we're good to land.

@goodboy goodboy added cancellation SC teardown semantics and anti-zombie semantics debugger labels Aug 2, 2022
@goodboy goodboy merged commit 641ed7a into master Aug 3, 2022
@goodboy goodboy deleted the signint_saviour branch August 3, 2022 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cancellation SC teardown semantics and anti-zombie semantics debugger testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants