LSP icon indicating copy to clipboard operation
LSP copied to clipboard

Wrong SessionView destroyed on restarting session

Open rchl opened this issue 2 years ago • 3 comments

Describe the bug

I've noticed that triggering "Restart server" on a file with active LSP-rust-analyzer session typically results in didClose being sent to the newly created session which is probably also the reason that inlay hints don't always show up properly after such action.

It looks like the culprit is the code that triggers after old session was destroyed but after new session has already started. This then destroys SessionView based on config name which obviously is not a good key for differentiating two instances of the Session for the same server.

https://github.com/sublimelsp/LSP/blob/346599a4fd00ba373c6b67f71e6fe41d3366e2a0/plugin/documents.py#L222-L228

To Reproduce

  1. Install LSP-rust-analyzer
  2. Open some *.rs file
  3. LSP: Restart session
  4. Watch the logs

Environment (please complete the following information):

  • Sublime Text version: 4150
  • LSP version: latest commit as of now

rchl avatar May 15 '23 21:05 rchl

Also this can result in auto complete triggers being removed from view settings due to those being out of order:

SessionView._clear_auto_complete_triggers
SessionView._clear_auto_complete_triggers
SessionView._setup_auto_complete_triggers
SessionView._clear_auto_complete_triggers

rchl avatar Jun 02 '23 22:06 rchl

I think this is also the cause for infinite initialize -> exit loops when you move a tab between windows and if it was the last tab of the session (doesn't happen all the time, but more often than not).

jwortmann avatar Jun 02 '23 23:06 jwortmann

Two ways of addressing this come to mind:

  1. ensure that only one instance of given config is running at the time (fully shut down one before starting another one)
  2. have unique/random IDs for each session

In the second case, we'd still have to use config name for at least auto complete triggers settings since on restarting ST we need to be able to lookup existing settings.

rchl avatar Jun 03 '23 15:06 rchl