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

Add support for exiting immediately from a shutdown() request. #1070

Merged
merged 1 commit into from
Jun 25, 2021

Conversation

rgrunber
Copy link
Contributor

LemMinX (at least for those adopting vscode-languageclient 7.x) will not exit immediately after the client shuts down, and relies on the parent process watcher, which can take some time.

  • In languageclient 7.x, the client fails to send the necessary exit()
    once a shutdown() response is received from the language server
  • When client defines shouldLanguageServerExitOnShutdown as true, the
    language server will exit immediately after the shutdown request

Signed-off-by: Roland Grunberg [email protected]

The delay ensures the shutdown() response will be received by the client.

@rgrunber
Copy link
Contributor Author

@datho7561

@datho7561 datho7561 self-requested a review June 25, 2021 14:43
@datho7561 datho7561 added this to the 0.17.1 milestone Jun 25, 2021
Copy link
Contributor

@datho7561 datho7561 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a changelog, since we will need to do a 0.17.1 release for this

datho7561 added a commit to datho7561/vscode-xml that referenced this pull request Jun 25, 2021
Supports the capability, which means that language server shuts down
when `shutdown` is received instead of when `exit` is received.

See eclipse/lemminx#1070

Signed-off-by: David Thompson <[email protected]>
datho7561 added a commit to datho7561/vscode-xml that referenced this pull request Jun 25, 2021
Supports the capability, which means that language server shuts down
when `shutdown` is received instead of when `exit` is received.

See eclipse/lemminx#1070

Signed-off-by: David Thompson <[email protected]>
- In languageclient 7.x, the client fails to send the necessary exit()
  once a shutdown() response is received from the language server
- When client defines shouldLanguageServerExitOnShutdown as true, the
  language server will exit immediately after the shutdown request
- Adjust CHANGELOG for a 0.17.1 release

Signed-off-by: Roland Grunberg <[email protected]>
@datho7561 datho7561 merged commit d13564a into eclipse:master Jun 25, 2021
datho7561 added a commit to datho7561/vscode-xml that referenced this pull request Jun 25, 2021
Supports the capability, which means that language server shuts down
when `shutdown` is received instead of when `exit` is received.

See eclipse/lemminx#1070

Signed-off-by: David Thompson <[email protected]>
@angelozerr
Copy link
Contributor

I have not tested but after debugging vscode languageclient 7.0.0 , the problem occured in shutdown process: It called:

  • connection.exit()
  • connection.end() without waiting for exit process.

I have not tested, but I think this commit microsoft/vscode-languageserver-node@6eef6d2#diff-1442f78812d67be25f18d3b5d82e8b4191ae2f939a900eac32d4360222c5d845R3349 should fix the issue.

IMHO, once microsoft/vscode-languageserver-node@6eef6d2#diff-1442f78812d67be25f18d3b5d82e8b4191ae2f939a900eac32d4360222c5d845R3349 will be released, we should test it and remove the shouldLanguageServerExitOnShutdown capability

Anyway thanks @rgrunber for your patch!

@rgrunber rgrunber deleted the fix-shutdown branch June 25, 2021 18:24
datho7561 added a commit to redhat-developer/vscode-xml that referenced this pull request Jun 25, 2021
Supports the capability, which means that language server shuts down
when `shutdown` is received instead of when `exit` is received.

See eclipse/lemminx#1070

Signed-off-by: David Thompson <[email protected]>
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

Successfully merging this pull request may close these issues.

3 participants