R crashes if Positron reattaches after busy

jmcphers commented 2 weeks ago

System details:

Positron and OS details:

Positron Version: 2024.11.0 (Universal) build 23
Code - OSS Version: 1.93.0
Commit: 9c57708e62a1ce5db9f26a4de3e5dc2dca168724
Date: 2024-10-09T02:45:16.388Z
Electron: 30.4.0
Chromium: 124.0.6367.243
Node.js: 20.15.1
V8: 12.4.254.20-electron.0
OS: Darwin arm64 23.6.0

Interpreter details:

Any R.

Describe the issue:

If Positron tries to reattach to R when R is busy, R panics and crashes.

Steps to reproduce the issue:

Ensure the Jupyter supervisor is enabled.
Run a long-running command in R, e.g. Sys.sleep(30).
Reload the window.

Positron waits to attach to R until it is finished executing the sleep command (expected). However, once attached, R crashes with the exit code -1.

https://github.com/user-attachments/assets/3d40dcb6-ab91-4f43-aef1-26e2657c5113

Expected or desired behavior:

No crash.

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

Yes:

[R]   2024-10-09T15:38:51.490231Z ERROR  Panic! In file 'crates/ark/src/interface.rs' at line 1632: called `Result::unwrap()` on an `Err` value: SendError(Kernel(DidChangeConsoleInputs(ConsoleInputs { console_scopes: [[".Random.seed"], [".__rstudio_ns__.", ".rs.api.documentNew", ".rs.api.documentSave", ".rs.api.documentSaveAll", [...]...  "utils"] })))
[R] 
[R] Backtrace:
[R]    0: std::backtrace::Backtrace::create
[R]    1: ark::main::{{closure}}::{{closure}}
[R]    2: ark::main::{{closure}}
[R]    3: std::panicking::rust_panic_with_hook
[R]    4: std::panicking::begin_panic_handler::{{closure}}
[R]    5: std::sys::backtrace::__rust_end_short_backtrace
[R]    6: _rust_begin_unwind
[R]    7: core::panicking::panic_fmt
[R]    8: core::result::unwrap_failed
[R]    9: ark::interface::RMain::refresh_lsp
[R]   10: ark::interface::RMain::read_console
[R]   11: harp::exec::try_catch::callback
[R]   12: R_withCallingErrorHandler
[R]              at /Volumes/Builds/R4/R-4.3.3/src/main/errors.c:2613:16
[R]   13: harp::exec::top_level_exec::callback
[R]   14: R_ToplevelExec
[R]              at /Volumes/Builds/R4/R-4.3.3/src/main/context.c:799:2
[R]   15: harp::exec::r_sandbox
[R]   16: _r_read_console
[R]   17: Rf_ReplIteration
[R]              at /Volumes/Builds/R4/R-4.3.3/src/main/main.c:210:10
[R]   18: R_ReplConsole
[R]              at /Volumes/Builds/R4/R-4.3.3/src/main/main.c:314:11
[R]   19: run_Rmainloop
[R]              at /Volumes/Builds/R4/R-4.3.3/src/main/main.c:1200:5
[R]   20: ark::interface::RMain::start
[R]   21: ark::main
[R]   22: std::sys::backtrace::__rust_begin_short_backtrace
[R]   23: std::rt::lang_start::{{closure}}
[R]   24: std::rt::lang_start_internal
[R]   25: _main
[R] 
[R]     at crates/ark/src/main.rs:282
[R]

lionel- commented 1 week ago

The LSP server is dropped when the connection closes, which in turn closes all transmission channels to the LSP. So when read_console() refreshes LSP inputs by sending data with an unwrap, we crash.

We could implement a teardown process for LSP-related state owned by the kernel. It would set the transmission channel back to None.
And we should not unwrap send failures. This channel is not connected to anything vital to the kernel so we should just log if unexpectedly closed.

lionel- commented 1 week ago

Actually Ark already handles the LSP reconnection cycle correctly. The problem arises when there are multiple LSP connections at the same time. We're a bit far off from supporting multiple connections but we probably don't have to right now since the jupyter-adapter path doesn't have this issue with refreshes while busy. (But when that happens we should at least panic earlier with a better error message, or ignore the request.)

Ark is getting redundant requests to start the LSP from kallichore. I also see a ton of requests to start the UI comms, e.g.:

[R]   2024-10-10T16:30:24.846565Z  INFO  Received request for open comms: JupyterMessage { zmq_identities: [[114, 45, 54, 51, 48, 50, 57, 97, 98, 100], [114, 45, 54, 51, 48, 50, 57, 97, 98, 100]], header: JupyterHeader { msg_id: "250f1e6813", session: "r-63029abd", username: "lionel", date: "2024-10-10T16:30:24.845Z", msg_type: "comm_info_request", version: "5.3" }, parent_header: None, content: CommInfoRequest { target_name: "positron.help" } }
[R]     at crates/amalthea/src/socket/shell.rs:275
[R] 
[R]   2024-10-10T16:30:24.846660Z  INFO  Sending UI message to frontend: Event(WorkingDirectory(WorkingDirectoryParams { directory: "~/Sync/Projects/R/r-lib/carrier" }))
[R]     at crates/ark/src/kernel.rs:132
[R] 
[R]   2024-10-10T16:30:24.846808Z ERROR  Error receiving Positron event; closing event listener: RecvError
[R]     at crates/ark/src/ui/ui.rs:70
[R] 
[R]   2024-10-10T16:30:24.846826Z  INFO  Comm channel opened; there are now 277 open comms
[R]     at crates/amalthea/src/comm/comm_manager.rs:118
[R]

posit-dev / positron