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

Fix inability to interrupt processes on Windows #12137

Merged
merged 16 commits into from Feb 27, 2020

Conversation

itamarst
Copy link
Contributor

@itamarst itamarst commented Feb 21, 2020

Fixes #3400

(or at least fixes the Windows subprocesses variant; there's a bunch of stuff in there that is probably separate tickets—at least pdb was moved out into its own).

It appears that in addition to wait() being uninterruptible (https://bugs.python.org/issue28168), so is read()ing from the subprocess.

Reading is therefore pushed off to threads, and wait() replaced with the equivalent poll()-based loop. If the user interrupts the process, it is killed.

This is demonstrated by a unit test that previously failed, and now passes. Still remaining for me to do is actually test this fix on Windows from Jupyter; I'm waiting for a VM to unpack. If a reviewer wants to try this out themselves, by all means do so :)

@itamarst
Copy link
Contributor Author

@itamarst itamarst commented Feb 21, 2020

I now have a process interrupting test that simulates how ipykernel handles interrupts, and it demonstrates that system()-run proceses:

  1. Exit quickly on Linux in response to the interrupt.
  2. Ignore the interrupt on Windows.

@itamarst
Copy link
Contributor Author

@itamarst itamarst commented Feb 21, 2020

It appears the subprocess also becomes uninterruptible while reading, not just during wait().

@itamarst itamarst changed the title [WIP] Attempt at fixing inability to interrupt processes Fix inability to interrupt processes on Windows Feb 26, 2020
@itamarst itamarst marked this pull request as ready for review Feb 26, 2020
@itamarst
Copy link
Contributor Author

@itamarst itamarst commented Feb 26, 2020

Investigating the Travis failure.

@Carreau Carreau added this to the 7.13 milestone Feb 27, 2020
@Carreau
Copy link
Member

@Carreau Carreau commented Feb 27, 2020

Well you seem to know what you are doing and have a windows machine. I can't test, but the code seem reasonable. Thanks !

@Carreau Carreau merged commit 1a01864 into ipython:master Feb 27, 2020
2 checks passed
meeseeksmachine pushed a commit to meeseeksmachine/ipython that referenced this issue Feb 27, 2020
Carreau added a commit that referenced this issue Feb 27, 2020
…137-on-7.x

Backport PR #12137 on branch 7.x (Fix inability to interrupt processes on Windows)
@amrita112
Copy link

@amrita112 amrita112 commented Jun 20, 2020

Should this be working in IPython version 7.15.0? I couldn't get it to work.
I ran the following in a jupyter notebook cell:
input()
and then tried to interrupt the kernel, but couldn't.

@itamarst
Copy link
Contributor Author

@itamarst itamarst commented Jun 21, 2020

@amrita112 you also need the newest IPykernel.

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

Successfully merging this pull request may close these issues.

3 participants