R exists unexpectedly when calling file.edit() many times

[EmilHvitfeldt]

Positron Version:

Positron Version: 2023.11.0 (Universal) build 1319
Code - OSS Version: 1.84.0
Commit: 7a0546f
Date: 2023-11-20T03:10:05.995Z
Electron: 25.9.2
ElectronBuildId: undefined
Chromium: 114.0.5735.289
Node.js: 18.15.0
V8: 11.4.183.29-electron.0
OS: Darwin arm64 22.6.0

Steps to reproduce the issue:

1. run a variation of fs::dir_ls(regexp = "qmd$") |> purrr::walk(file.edit)

Kapture.2023-11-23.at.09.23.17.mp4

What did you expect to happen?

For it not to crash. I use this idiom all the time when i write and work

Were there any error messages in the output or Developer Tools console?

[lionel-]

Reprex:

path <- fs::dir_create(tempfile())

for (i in 1:50) {
  fs::file_create(fs::path(path, i))
}

fs::dir_ls(path) |> purrr::walk(file.edit)

Or more simply:

path1 <- file.create(tempfile())
path2 <- file.create(tempfile())
repeat { file.edit(path1); file.edit(path2) }

Useful actions in the right-click menu of an editor tab after opening many temp files: Close all others / Close to the right

That's weird I could reproduce the first time, and then in fresh sessions I no longer can reproduce.

[DavisVaughan]

See also #857

[DavisVaughan]

Somewhat related to #1956 as well

[DavisVaughan]

I've been investigating this and can reproduce it pretty well with @lionel-'s fs example. Notably, the problem goes away when you insert an early exit in the (seemingly unrelated) diagnostics code right before this r_task()

https://github.com/posit-dev/amalthea/blob/1b7018d2e35203e7becfbc46a09b48d8b6efe087/crates/ark/src/lsp/diagnostics.rs#L169

Here is what I think is happening:

• ps_editor() is called a few times and things are working as expected
• In the meantime, the Console is updated since you just ran some code which cleared it, and a did_change() LSP event is registered for the console. This starts enqueue_diagnostics() and a tokio thread with a 1 second delay, after which diagnostics are actually generated.
• ps_editor() is called on, say, Document 5. It runs show_document(5).await, and waits for confirmation that the document open request has been sent. Assume that await happens to coincide with the end of the 1 second delay over in the Console's diagnostic refresh request, so tokio switches to generating the diagnostics.
• generate_diagnostics() is then run, but this calls into R using an r_task()! This is unable to run because:
  • The r_task() is queued up to be run on the R main thread next time we loop through read_console()
  • But ps_editor() is still awaiting, locking up the R main thread. It can't be freed until the next await call in enqueue_diagnostics(), which won't ever come since that is waiting on ps_editor().
  • So we are deadlocked

[lionel-]

@DavisVaughan Good findings!

One way to fix this would be to introduce a mechanism to "yield" from ps_editor to other tasks while awaiting for LSP routines to complete their tasks.

We'd replace the block_on by a Crossbeam select that would execute incoming R tasks until the Rust future has resolved and notified select via a special channel. That channel could pass on the future result though that doesn't seem needed here. Once notified, we break from the select to unblock ourselves and let ps_editor return control to the user.

I'm not familiar with how tokio/futures work in Rust but I've read that Rust futures are lazy and need to be actively polled. It's not like Javascript where you create a promise and it's immediately scheduled on the event loop and goes on to live its own life even if you don't need the result. So there might be a need to keep alive the future until it has resolved, or keep polling it from the select, or block on it from another thread, or something like that.

[juliasilge]

In Positron 2024.01.0 (Universal) build 141 I can edit SO MANY FILES 😳

so-many-files.mov

Looking fantastic! 🎉