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

Windows: Ctrl + C is unreliable to interrupt a readline() #5710

Open
DavisVaughan opened this issue Dec 12, 2024 · 5 comments
Open

Windows: Ctrl + C is unreliable to interrupt a readline() #5710

DavisVaughan opened this issue Dec 12, 2024 · 5 comments
Labels
area: console Issues related to Console category. area: kernels Issues related to Jupyter kernels and LSP servers lang: r
Milestone

Comments

@DavisVaughan
Copy link
Contributor

Ctrl + C works sometimes to get you out of readline, but not reliably

ctrlc.mov

When it works

2024-12-12 09:53:31.649 [debug] State: busy => busy (interrupt_request)
2024-12-12 09:53:31.649 [debug] State: busy => idle (interrupt_request)
2024-12-12 09:53:31.650 [debug] State: idle => idle (execute_request)
[R]   2024-12-12T14:53:31.642495Z  INFO  Received interrupt request, asking kernel to stop: JupyterMessage { zmq_identities: [[114, 45, 56, 53, 102, 100, 54, 97, 102, 97]], header: JupyterHeader { msg_id: "649f54555a", session: "r-85fd6afa", username: "dvaug", date: "2024-12-12T14:53:31.642Z", msg_type: "interrupt_request", version: "5.3" }, parent_header: None, content: InterruptRequest }
[R]     at crates\amalthea\src\socket\control.rs:152
[R] 
[R]   2024-12-12T14:53:31.642522Z  INFO  Received interrupt request
[R]     at crates\ark\src\control.rs:58
[R] 
[R]   2024-12-12T14:53:31.642537Z TRACE  Sending 'interrupt_reply' message (reply to 'interrupt_request') via Control socket
[R]     at crates\amalthea\src\wire\wire_message.rs:196
[R] 
[R]   2024-12-12T14:53:31.642556Z TRACE  Received interrupt signal in StdIn
[R]     at crates\amalthea\src\socket\stdin.rs:142
[R] 
[R]   2024-12-12T14:53:31.642601Z TRACE  Waiting for control messages
[R]     at crates\amalthea\src\socket\control.rs:58
[R] 
[R]   2024-12-12T14:53:31.642602Z TRACE  Sending 'status/busy' message (reply to 'interrupt_request') via IOPub socket
[R]     at crates\amalthea\src\wire\wire_message.rs:196
[R] 
[R]   2024-12-12T14:53:31.642711Z TRACE  Sending 'status/idle' message (reply to 'interrupt_request') via IOPub socket
[R]     at crates\amalthea\src\wire\wire_message.rs:196
[R] 
[R]   2024-12-12T14:53:31.644701Z TRACE  Interrupting `ReadConsole()`
[R]     at crates\ark\src\interface.rs:1938
[R] 
[R]   2024-12-12T14:53:31.644805Z  INFO  Sending message to UI comm: Event(Busy(BusyParams { busy: false }))
[R]     at crates\ark\src\ui\sender.rs:50
[R] 
[R]   2024-12-12T14:53:31.644842Z TRACE  prompt_info(): n_frame = '0'
[R]     at crates\ark\src\interface.rs:839
[R] 
[R]   2024-12-12T14:53:31.644849Z TRACE  R prompt: > 
[R]     at crates\ark\src\interface.rs:681
[R] 
[R]   2024-12-12T14:53:31.644855Z  INFO  Sending message to UI comm: Event(PromptState(PromptStateParams { input_prompt: "> ", continuation_prompt: "+ " }))
[R]     at crates\ark\src\ui\sender.rs:50
[R] 
[R]   2024-12-12T14:53:31.644872Z TRACE  Got R prompt '> ', completing execution
[R]     at crates\ark\src\interface.rs:1402

When it doesnt

2024-12-12 09:54:09.516 [debug] State: busy => busy (interrupt_request)
2024-12-12 09:54:09.516 [debug] State: busy => idle (interrupt_request)
[R]   2024-12-12T14:54:09.503112Z  INFO  Received interrupt request, asking kernel to stop: JupyterMessage { zmq_identities: [[114, 45, 56, 53, 102, 100, 54, 97, 102, 97]], header: JupyterHeader { msg_id: "22d88ddd73", session: "r-85fd6afa", username: "dvaug", date: "2024-12-12T14:54:09.502Z", msg_type: "interrupt_request", version: "5.3" }, parent_header: None, content: InterruptRequest }
[R]     at crates\amalthea\src\socket\control.rs:152
[R] 
[R]   2024-12-12T14:54:09.503180Z  INFO  Received interrupt request
[R]     at crates\ark\src\control.rs:58
[R] 
[R]   2024-12-12T14:54:09.503229Z TRACE  Sending 'interrupt_reply' message (reply to 'interrupt_request') via Control socket
[R]     at crates\amalthea\src\wire\wire_message.rs:196
[R] 
[R]   2024-12-12T14:54:09.503261Z TRACE  Received interrupt signal in StdIn
[R]     at crates\amalthea\src\socket\stdin.rs:142
[R] 
[R]   2024-12-12T14:54:09.503385Z TRACE  Waiting for control messages
[R]     at crates\amalthea\src\socket\control.rs:58
[R] 
[R]   2024-12-12T14:54:09.503458Z TRACE  Sending 'status/busy' message (reply to 'interrupt_request') via IOPub socket
[R]     at crates\amalthea\src\wire\wire_message.rs:196
[R] 
[R]   2024-12-12T14:54:09.503901Z TRACE  Sending 'status/idle' message (reply to 'interrupt_request') via IOPub socket
[R]     at crates\amalthea\src\wire\wire_message.rs:196
@DavisVaughan
Copy link
Contributor Author

The most obvious thing to note is that we dont see Interrupting ReadConsole() in the failing case


Here is a guess. Here is our event loop in read_console().

https://github.com/posit-dev/ark/blob/3b8ff6112d14f1b78640e4db55c82754c0b362bf/crates/ark/src/interface.rs#L766-L840

Note that we check for interrupts at the top of the loop each time. Note this comment

            // ... This needs to happen before
            // `process_events()`, particularly on Windows, because it
            // calls `R_ProcessEvents()`, which checks and resets
            // `UserBreak`, but won't actually fire the interrupt b/c
            // we have them disabled, so it would end up swallowing the
            // user interrupt request.

So if process_events() is called, then in theory that resets UserBreak and it looks like an interrupt hasn't occurred

Imagine we are sitting in the select! waiting on a reply for readline, expecting it to come through stdin_reply_rx. The user interrupts and UserBreak=true, so stdin_reply_rx is never going to get a reply, and instead we drop through to this case after the 200ms timeout

                default(Duration::from_millis(200)) => {
                    unsafe { Self::process_events() };
                }

After this we'd circle back around to the top of the loop and look at interrupts_pending(), but if the comment is to be believed, that process_events() call reset UserBreak to false and no interrupt was handled.

I think all this would kind of explain why sometimes it works.

If we can confirm this, a fix may be:

  • Check for interrupts in the default case before process_events()
  • Or maybe stdin_reply_rx should receive a new event variant called Interrupted or something, and handle it appropriately

@lionel-
Copy link
Contributor

lionel- commented Dec 12, 2024

After this we'd circle back around to the top of the loop and look at interrupts_pending(), but if the comment is to be believed, that process_events() call reset UserBreak to false and no interrupt was handled.

I think interrupt_pendings() looks at R_interrupts_pending which does get set by process_events() though. In other words, the latter propagates (and resets) UserBreak as R_interrupts_pending.

@DavisVaughan
Copy link
Contributor Author

@lionel- the windows defn of interrupts-pending is different

pub fn interrupts_pending() -> bool {
    unsafe { libr::get(UserBreak) == Rboolean_TRUE }
}

@lionel-
Copy link
Contributor

lionel- commented Dec 12, 2024

oh then that would certainly explain it!

@jennybc jennybc added area: console Issues related to Console category. lang: r area: kernels Issues related to Jupyter kernels and LSP servers labels Dec 12, 2024
@juliasilge
Copy link
Contributor

If it's more appropriate to move this to the ark repo, please feel free!

@juliasilge juliasilge added this to the Future milestone Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: console Issues related to Console category. area: kernels Issues related to Jupyter kernels and LSP servers lang: r
Projects
None yet
Development

No branches or pull requests

4 participants