fix(lsp): close timer when client exits#36795
Conversation
46d99de to
76e1d47
Compare
Also, don't start the timer at all when a previous shutdown failed, as in this case a forced shutdown is used and no timer is needed. This fixes most of the delays caused by neovim#36750. The delays caused by neovim#36378 still seem to remain.
76e1d47 to
6e37c57
Compare
|
Just fyi: #36783 and discussion in #36752 (comment) is somewhat related. Imho we should change the default back to |
| --- @field _otf_enabled boolean? | ||
| --- | ||
| --- Timer for stop() with timeout. | ||
| --- @field private _shutdown_timer uv.uv_timer_t? |
There was a problem hiding this comment.
name it _exit_timer for consistency with _on_exit / _on_exit_cbs / exit_timeout ?
(_graceful_shutdown_failed should perhaps also be renamed)
There was a problem hiding this comment.
Yes, but
shutdownmatches the name of the LSP request used when stopping the server.- I think
shutdownandexitare two different things.shutdownis a client action that affects the server, whileexitis a server action.
There was a problem hiding this comment.
It does, but in our layer we refer to "exit" and "stop". It's true that we send a "shutdown" to the server, but that is somewhat of an implementation detail.
Some of it overlaps, so it's somewhat messy. But at least the specific case of this timer is tracking "stop" and/or "exit". Could name this _stop_timer too. Mostly, it's useful to use a common prefix so all of these related things are grouped.
justinmk
left a comment
There was a problem hiding this comment.
The naming nit is not a blocker
Also, don't start the timer at all when a previous shutdown failed, as in this case a forced shutdown is used and no timer is needed. This fixes most of the delays caused by neovim#36750. The delays caused by neovim#36378 still seem to remain.
Also, don't start the timer at all when a previous shutdown failed, as
in this case a forced shutdown is used and no timer is needed.
This fixes most of the delays caused by #36750.
The delays caused by #36378 still seem to remain.