sublimelsp / LSP

Client implementation of the Language Server Protocol for Sublime Text
https://lsp.sublimetext.io/
MIT License
1.64k stars 182 forks source link

shutdown sent between initialize and initialized #668

Closed PerMildner closed 5 years ago

PerMildner commented 5 years ago

Sublime Text (sometimes?) sends a shutdown request after initialize but before subsequent initialized and didOpen, see attached log. This is not a valid sequence of messages.

After shutdown the server should only accept exit so it is effectively stopped. The specification says "If a server receives requests after a shutdown request those requests should be errored with InvalidRequest".

Please include in your report:

LSP: Communication to server closed, exiting LSP: transport failed LSP: Couldn't determine project directory since no folders are open! Using /Users/perm/sicstus/lsptestfolder as a fallback. reloading settings Packages/User/Preferences.sublime-settings reloading settings Packages/User/Preferences.sublime-settings reloading settings Packages/User/Preferences.sublime-settings reloading settings Packages/User/Preferences.sublime-settings

tomv564 commented 5 years ago

The shutdown message is sent because LSP thinks you have opened a different project.

LSP is also logging that you have not created a sublime project for your 'lsptestproject' folder. Create a project for this folder and I bet this issue will disappear. LSP tries to handle support project-less operation on a best-effort basis, but bugs like these are expected.

Explanation: LSP detects a supported file (foo.pl) for which it starts a session in lsptestfolder and then an unsaved file is opened for which project_path is None. When project path changes from we assume you've stopped working in lsptestfolder, so we end that session.

This logic works reliably with sublime projects but needs substantial changes for project-less windows. If someone can find a really elegant way to manage sessions then please propose a design in a new issue or PR.

tomv564 commented 5 years ago

I feel LSP does too little to warn users about project-less mode. Perhaps we can either:

PerMildner commented 5 years ago

The problem is not so much the shutdown message (and the missing exit message that ought to follow the shutdown). The problem is that didOpen is sent to a server that Sublime Text knows it has shut down. This is a LSP protocol violation.

If a proper shutdown (shutdown + exit) was followed by a proper startup (launch server, initialize, ...) before the didOpen, then it would perhaps not work perfectly for project-less files, but at least it would adhere to the LSP protocol. As it now stands a conformant server will just report a InvalidRequest error for all subsequent requests.

Perhaps it would be possibly to simply start one server instance for each project-less file? Or, not use the single server instance except for the first file, and not use LSP for subsequent files.

Other clients seems to just default to the surrounding folder as rootURI (and workspace folder) so everything works OK as long as all files are in, or below, that initial folder. Then, there is the workspace/didChangeWorkspaceFolders LSP-message that perhaps could be used to just add any new containing folder. I know nothing about ST projects, though.

PerMildner commented 5 years ago

Looking some more at this. It seems exit is sent, at least sometimes, some time after the shutdown, but I do not see it always (so, there is not a fundamental bug that "exit is not sent after shutdown").

Looking at the source (*) I suspect a thread synchronization issue. I see nothing that prevents the didOpen (which seems to be posted from its own async thread) from getting sent to the server after a shutdown has been sent.

Also, once, when flipping around among the file tabs in Sublime Text, which triggers the shutdown/restart/whatever of the server, it appeared that several shutdown messages was buffered somewhere and then all was received by the same server. Since there are so many moving parts, and timing is involved, this is really hard to troubleshoot, so this particular symptom may be a false alarm.

(*) Be aware that I know nothing about the Sublime Text API, nor of Python, so this is just informed guesswork.

tomv564 commented 5 years ago

Yeah, it's hard to reason about it - improving behaviour in single-file mode is the first priority as it can be hiding other bugs as well. There is no "shutting down" state for the session, but that would prevent a late didOpen being sent to it. Let's keep this issue open to fix the single-file mode first, then look at the behaviour again.

tomv564 commented 5 years ago

I think with #713 some of the unintended shutdowns should have disappeared. From your logs, I am not sure why your spider-nc config is being started for a preferences edit:

window 3 requests spider-nc for /Users/perm/Library/Application Support/Sublime Text 3/Packages/Default/Preferences.sublime-settings

I'll close the issue now without adding a "shutting down" state to sessions - assuming LSP will rarely have a legitimate reason to shutdown sessions that were just started.