posit-dev / positron

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

Ark: `usethis::edit_r_environ()` seems to freeze on Windows #1956

Closed DavisVaughan closed 9 months ago

DavisVaughan commented 10 months ago

It seems to work ok in some cases, but not others.

It looks like it gets stuck here: https://github.com/posit-dev/amalthea/blob/931cebea670c9b24c6f943ddda610d19e1f00854/crates/ark/src/lsp/editor.rs#L40

An explicit utils::file.edit("D:/Users/davis-vaughan/Documents/.Renviron") seems work fine though, so I'm not entirely sure what the minimal reprex is

DavisVaughan commented 10 months ago
Screenshot 2023-12-15 at 6 10 57 PM

It often does open, but hangs

DavisVaughan commented 9 months ago

@lionel- and I are fairly certain this is due to sharing 1 LSP Client across 2 different tokio Runtimes, which seems to be able to lock up the Client and is generally probably not a great thing to do.

i.e. start_lsp() has an implicit runtime through the #[tokio::main] attribute (which creates and uses the client, and stores it as a global lsp_client), and then ps_editor() starts an explicit Runtime (which then uses lsp_client).

I think to do this we'd have to at least have a Mutex around all accesses to Client (I'm not sure if we can control how tower lsp accesses the Client though).

An alternative approach for now is to manually generate the LSP Runtime and make it a global variable as well, ensuring that 1 Runtime is used anywhere we call the LSP Client. That seems to fix the Windows issue from what i can tell.

jennybc commented 9 months ago

I just verified that usethis::edit_r_environ() seems to be behaving nicely on Windows.

https://github.com/posit-dev/positron/assets/599454/7e6cbe61-eaae-4ddb-9b57-f12be13f2df4

Note: This video is from a dev build, as I was having generalized problems with the most recent Windows release build from GitHub, i.e. general unhappiness with the interpreter.