posit-dev / positron

Positron, a next-generation data science IDE
https://positron.posit.co
Other
2.76k stars 84 forks source link

Interrupt of the python console doesn't work on windows #4604

Open jonvanausdeln opened 2 months ago

jonvanausdeln commented 2 months ago

System details:

Windows 11

Positron and OS details:

All builds

Interpreter details:

Python 3.12.2 (but version doesn't matter)

Describe the issue:

In the python console, interrupting a long running process doesn't work. This could be an issue for users using packages like Vetiver

Steps to reproduce the issue:

  1. Start a python cosole
  2. In the console run
    >>> import time
    >>> time.sleep(60)
  3. Click the interrupt button (red box)

This doesn't stop the process, instead I get image

Expected or desired behavior:

Process to be interrupted

Were there any error messages in the UI, Output panel, or Developer Tools console?

[Python] [PositronIPKernelApp] ERROR | Interrupt message not supported on Windows
jmcphers commented 2 months ago

Without digging into the code too much, I would speculate that this is caused by us relying on sending the interrupt_request Jupyter message on all platforms to interrupt Python. Not all kernels support this message on all platforms; at least according to spec, the kernel should declare (in its kernelspec) an interrupt_mode indicating whether interrupts can use this message or whether they need to send an OS-level signal.

https://jupyter-client.readthedocs.io/en/latest/kernels.html#kernel-specs

We don't read kernelspecs in Positron since we don't require that the kernel be pre-registered with Jupyter in any way, so we probably just need some built-in knowledge, along with the ability to deliver an OS signal to interrupt if needed.

juliasilge commented 2 months ago

We think we may solve this with the new Jupyter kernel supervisor.

jmcphers commented 1 month ago

I have a candidate fix for this in the new kernel supervisor; it now sends SIGINT to kernels that don't support an interrupt message.

jmcphers commented 2 weeks ago

Update: The candidate fix doesn't work, because (and I totally should have known this already) SIGINT isn't supported on Windows.

https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/signal?view=msvc-170

Instead we need to do some very Windows-specific stuff with an interrupt handle. Here's how Jupyter Client does it:

https://github.com/jupyter/jupyter_client/blob/5.2.3/jupyter_client/win_interrupt.py

jmcphers commented 2 weeks ago

Some more research..

Interrupting time.sleep(...) also does not work in VS Code's notebooks.

Image

Image

The interrupt handler used by the Jupyter extension is here; it uses the same technique and environment variable (JPY_INTERRUPT_EVENT) as the Jupyter client. https://github.com/microsoft/vscode-jupyter/blob/fb42c2069afd743d23a71c2fd340295909dfb58a/pythonFiles/vscode_datascience_helpers/kernel_interrupt_daemon.py

So:

jmcphers commented 1 week ago

I have a branch in the supervisor repo with an implementation of interrupts that uses Windows events, but haven't been able to get it to work (no errors, but the signal raised on the supervisor is not received in the spawned kernel for reasons that aren't clear yet).