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

Improve lifecycle management of LSP #622

Open
DavisVaughan opened this issue Nov 7, 2024 · 2 comments
Open

Improve lifecycle management of LSP #622

DavisVaughan opened this issue Nov 7, 2024 · 2 comments

Comments

@DavisVaughan
Copy link
Contributor

@lionel- and I were looking at the LSP lifecycle a little more and realized that when the tower-lsp server exits here:

server.serve(service).await;

we should also unset the events channel in RMain that we sent it here:

r_task({
let events_tx = events_tx.clone();
move || {
let main = RMain::get_mut();
main.set_lsp_channel(events_tx);
}
});

because there is no longer anything it can send messages to.

The tower-lsp server will exit like this during a Positron developer refresh, so it is a decently common case. I think it is possible for us to get in a bad place if we don't clear the RMain sender.

@lionel-
Copy link
Contributor

lionel- commented Nov 8, 2024

See discussion in #617 (comment)

@lionel-
Copy link
Contributor

lionel- commented Nov 8, 2024

If we solve the issue of zombie extension hosts, it would also be good to detect whether an LSP is already running on comm_open. That would be an error as the client should first shut down the LSP.

Unfortunately comm_open is a notification, not a request, so there is no straightforward way of propagating the error. We might want to extend this message and allow it to be a request if a field is set. This would be advertised as a kernel capability and allow the client to wait for a result and know for sure whether an LSP was started.

@lionel- lionel- changed the title Clear events_tx in RMain on LSP shutdown Improve lifecycle management of LSP Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants