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

Dispose LSP's output channel on restart #841

Conversation

plemarquand
Copy link
Contributor

@plemarquand plemarquand commented Jun 5, 2024

When restarting SourceKit-LSP its output channel was not disposed, which caused a new entry to be added to the output channel list without removing the old one.

Fixes #839

When restarting SourceKit-LSP its output channel was not disposed, which
caused a new entry to be added to the output channel list without
removing the old one.
@adam-fowler
Copy link
Contributor

This is kind of dodgy, in that we are assuming the VSCode language client never calls the dispose on the outputChannel. Looking through the language client code it should be calling it, so I am a little confused as to why this call is needed.
https://github.com/microsoft/vscode-languageserver-node/blob/0226f6cd925d279dda2b777d25c5b69518ac0035/client/src/common/client.ts#L1414

@plemarquand
Copy link
Contributor Author

After some digging I understand what is happening here, and it does seem to be a bug in vscode-languageclient. The flow is:

  • The initial client is created, client's output channel ("OC1") is created.
  • User runs "Restart LSP Server"
  • client.stop is called, "OC1" is disposed, client._outputChannel is set to undefined
  • Cleanup continues, and the spawned LSP process is terminated
  • This finally block executes, listening for the LSP process to exit.
  • LSP exits with code 0, which is then logged here.
  • The outputChannel getter, seeing that this._outputChannel is undefined due to previous disposal, creates a new output channel ("OC2") that replaces the one that was disposed.
    • This output channel is what I'm disposing in this patch. Without this code, OC2 never gets disposed.
  • Finally, we create a new client, which creates a new output channel "OC3".

This leaves us with one more output channel than we started with.

@adam-fowler
Copy link
Contributor

adam-fowler commented Jun 6, 2024

After some digging I understand what is happening here, and it does seem to be a bug in vscode-languageclient. The flow is:

Do you want to add an issue for the vscode language client? Oh and add a comment here detailing the issue and that it should be reverted once a fix is in vscode-language-server

@plemarquand
Copy link
Contributor Author

Sounds good, I'll create that issue once I have received internal approval and link it back to here.

@plemarquand plemarquand merged commit 8171b24 into swiftlang:main Jun 6, 2024
8 checks passed
@plemarquand plemarquand deleted the dispose-lsp-output-channel-on-restart branch June 6, 2024 17:01
@plemarquand
Copy link
Contributor Author

Created microsoft/vscode-languageserver-node#1496 to track the root cause in vscode-language-server-node.

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.

Relaunching the LSP Server doesn't clean up the output sections
2 participants