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

Make IPython.core.debugger interruptible by KeyboardInterrupt #12168

Merged
merged 13 commits into from
Apr 23, 2020

Conversation

itamarst
Copy link
Contributor

@itamarst itamarst commented Mar 2, 2020

Together with a number of other PRs, this is a step towards addressing #10516

There are three issues:

  1. IPython.core.debugger would swallow KeyboardInterrupts and not let them get raised. This fixes that issue in a new class, InterruptiblePdb, so as not to change existing behavior.
  2. pdb had the same issue, and by default all debuggers in Jupyter kernel usage need to be interruptible by KeyboardInterrupt to fix this bug. Make pdb on Windows interruptible ipykernel#490 overrides pdb and the default debugger to use the new InterruptiblePdb.
  3. Finally, ipykernel's mechanism for interrupting subprocesses doesn't raise KeyboardInterrupt on Windows when waiting for input(). Fixed by Allow interrupting input() on Windows, as part of effort to make pdb interruptible ipykernel#498

@Carreau
Copy link
Member

Carreau commented Mar 3, 2020

Thanks, marking as 7.14.

There seem to be some unexpected interaction as IPython tests are failing, but they should not depend on PyZMQ. I'll have to dig into this.

@itamarst
Copy link
Contributor Author

itamarst commented Mar 3, 2020

I can take a look too, just, step 1 is convincing pyzmq maintainer to accept that PR (or convincing myself that PR is not necessary).

@Carreau
Copy link
Member

Carreau commented Mar 3, 2020

if that's @minrk then he can also merge the fixes here if necessary and likely do synchronized releases.

@itamarst
Copy link
Contributor Author

itamarst commented Mar 3, 2020

Ah, great, just need to convince him it's necessary :)

@itamarst
Copy link
Contributor Author

itamarst commented Mar 3, 2020

Looks like I was wrong and pyzmq is fine, so just down to these two PRs.

@itamarst
Copy link
Contributor Author

After further research, I have determined that the interrupt mechanism used by ipykernel on Windows doesn't interrupt pdb (or rather, I suspect, input()). Some research brought up this epic comment (https://stackoverflow.com/a/35792192) which explains how to make a signal handler for Windows that actually works, and I reproduced it in the test.

Unfortunately this will require some corresponding changes to ipykernel, and still has some... oddities. Like, I need to send the signal multiple times (perhaps there's a race condition? A flag that needs to be set?). Python 3.6 isn't working in AppVeyor so that suggests more issues. If I can just get it working on e.g. POSIX plus Python 3.8 Windows that will still be an improvement, so I will try to do that next.

…e issues

with with Windows, that will be addressed in ipykernel (which will grow its own test).
@itamarst
Copy link
Contributor Author

I switched to a simpler test that doesn't involve subprocesses. I will next try to write a process-interrupting test in an ipykernel branch, which is a better place to do it since it is the one that actually implements that logic, and see if I can fix the Windows issue I encountered there.

@itamarst
Copy link
Contributor Author

@Carreau OK I think this is ... plausibly maybe even correct and working now 😁

@Carreau
Copy link
Member

Carreau commented Apr 23, 2020

OK, let's try.

@Carreau Carreau merged commit 95d5171 into ipython:master Apr 23, 2020
meeseeksmachine pushed a commit to meeseeksmachine/ipython that referenced this pull request Apr 23, 2020
Carreau added a commit that referenced this pull request Apr 23, 2020
…168-on-7.x

Backport PR #12168 on branch 7.x (Make IPython.core.debugger interruptible by KeyboardInterrupt)
@tonyfast
Copy link
Contributor

Anyone have an idea as to while my pdb.Pdb class is being updated to IPython.core.debugger.InterruptiblePdb? I can't figure out where in the debugger phases this class is being forced to change. It is causing issues with running doctest in a shell session.

image

@itamarst
Copy link
Contributor Author

@tonyfast It happens in ipykernel.

@tonyfast
Copy link
Contributor

Is there an option or flag to not do this?

I don't want to use pdb and it is disrupting a testing pattern in the stdlib module which is harshing my buzz.

@tonyfast
Copy link
Contributor

Could this be something that the InteractiveShell handles with traitlets when shell.pdb is True?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants