Skip to content

send shutdown/exit to LSP server upon vim exit#783

Merged
yegappan merged 6 commits intoyegappan:mainfrom
castedo:fix-shutdown-exit
Mar 18, 2026
Merged

send shutdown/exit to LSP server upon vim exit#783
yegappan merged 6 commits intoyegappan:mainfrom
castedo:fix-shutdown-exit

Conversation

@castedo
Copy link
Contributor

@castedo castedo commented Mar 16, 2026

  • remove unnecessary wait loop after sending LSP exit
  • remove old unused global variable LSPServers

* remove unnecessary wait loop after sending LSP exit
* remove old unused global variable LSPServers
@castedo
Copy link
Contributor Author

castedo commented Mar 16, 2026

I'm not 100% sure, but I'm pretty sure that it is not needed and does nothing more than add a 2 seconds delay (which is why this wasn't called during vim exit). The LSP request of shutdown is already synchronous so I strongly suspect this provides all the delay that is needed (and I see pylsp respond VERY quickly). After that I don't think vim really needs to wait after the LSP exit getting sent.

@castedo
Copy link
Contributor Author

castedo commented Mar 16, 2026

that it is not needed

Sorry, that was ambiguous. By 'it' I mean the delay wait loop code that I remove in this PR (so that the function can be called upon vim exit without issue).

@yegappan
Copy link
Owner

I'm not 100% sure, but I'm pretty sure that it is not needed and does nothing more than add a 2 seconds delay (which is why this wasn't called during vim exit). The LSP request of shutdown is already synchronous so I strongly suspect this provides all the delay that is needed (and I see pylsp respond VERY quickly). After that I don't think vim really needs to wait after the LSP exit getting sent.

This delay is needed when restarting a language server using the :LspServer restart command. The restart command stops the server, waits for the server to stop and then starts the server again. Without the delay, when starting the server again, the previous instance of the server may still be running.

@castedo
Copy link
Contributor Author

castedo commented Mar 16, 2026

Oh my, I should address that. I am thinking I should fix this PR so there are two distinct kinds of shutdown/exit. A "fast" one that is only used for vim exit and the existing "slow" one that is used otherwise. @yegappan What do you think?

@yegappan
Copy link
Owner

Oh my, I should address that. I am thinking I should fix this PR so there are two distinct kinds of shutdown/exit. A "fast" one that is only used for vim exit and the existing "slow" one that is used otherwise. @yegappan What do you think?

Yes. Sounds good to me.

* renamed from StopAllServers since previously unused
* reverted autoload/lsp/lsp.vim to leave StopServer as is
@castedo
Copy link
Contributor Author

castedo commented Mar 16, 2026

OK, I've reverted StopServer with its wait/delay to be like main. This PR now has vim exit take a separate code path through a new "fast" function.

@castedo
Copy link
Contributor Author

castedo commented Mar 17, 2026

@yegappan I don't see any issues with this PR now. Is there anything you see that should be updated?

@castedo castedo marked this pull request as draft March 17, 2026 18:05
125ms timeout on synchronous 'shutdown' request for vim exit
@castedo castedo marked this pull request as ready for review March 17, 2026 21:00
@castedo
Copy link
Contributor Author

castedo commented Mar 17, 2026

@yegappan Here's a new proposal I've coded in the PR now: a sync call to shutdown with a 125ms timeout. Seems to me a good simple trade-off. Under normal circumstances servers will respond much faster than that and vim and server are doing a proper LSP protocol dance.

Under unusual circumstances, a user won't even notice since it won't be more than an eighth of a second before the whole subprocess is terminated anyway.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the Vim LSP plugin to send a fast shutdown + exit sequence to all running LSP servers during VimLeavePre, and removes an unused global server list in favor of using the existing filetype→server map.

Changes:

  • Add a VimLeavePre autocmd to trigger a fast shutdown/exit of all LSP servers.
  • Implement FastShutdownServer() / FastShutdownExitAllServers() using a short synchronous timeout for shutdown.
  • Remove the unused global LSPServers list.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
plugin/lsp.vim Hooks Vim exit (VimLeavePre) to trigger the new fast shutdown routine.
autoload/lsp/lsp.vim Implements fast shutdown/exit helpers and removes the unused LSPServers global.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@yegappan
Copy link
Owner

Thanks for the patch. Can you address the comments from the copilot review?

@yegappan
Copy link
Owner

I will look into adding a timeout parameter to the lsp_server.rpc() call in a later PR, so that it can be used directly.

@castedo
Copy link
Contributor Author

castedo commented Mar 18, 2026

Thanks for the patch. Can you address the comments from the copilot review?

Did you actually read the AI comments from the copilot review?

@yegappan
Copy link
Owner

Thanks for the patch. Can you address the comments from the copilot review?

Did you actually read the AI comments from the copilot review?

Yes. I did go through the comments added by copilot. As some of the comments are valid, I requested you to address them. Thanks for addressing the comments.

@yegappan yegappan merged commit b64d922 into yegappan:main Mar 18, 2026
2 checks passed
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