fix: defer ServerStatus.stopping until grace period expires#1554
fix: defer ServerStatus.stopping until grace period expires#1554SergejSalnikov wants to merge 1 commit into
Conversation
startStopTimer() was setting ServerStatus.stopping immediately when the lastDocumentDisconnectedTimeout timer started, not when the server actually began shutting down. This caused the LSP console to display "stopping pid:..." with a spinner for the entire grace period, even though the server was still fully functional and accepting connections. With the default 5-second timeout this was barely noticeable, but with longer timeouts (e.g. 180s for expensive-to-start servers) the UI was misleading — users could think the server was broken or unresponsive. Move updateStatus(ServerStatus.stopping) inside the alarm callback, just before stop(). The server now stays in 'started' status during the grace period and only transitions to 'stopping' when shutdown actually begins. FIX=redhat-developer#1553
|
If you do that you will never seen that server is stopping, right? |
|
I did not look into "stopping" logic. I though that it send terminate command and waits until server shuts down (which I assumed could take time). Maybe the state should be renamed from "stopping" into "idling"? |
|
"stopping" implies that there is some busy work is in progress. |
|
@SergejSalnikov IMHO I think stopping is the right name. The server is stopping.... I wonder if in your case you want never stop the server when all files are closed. If you want to do that you will need to create custom LSPClientFeatures and |
|
I want to have 3 min idling time before the server shuts itself down. |
|
Why not never stop the server? |
|
Because remote language server holds many dependent services such as clangd, golangd, typescript ls and others. |
|
And why you don't do my suggestion?
|
|
because if user doesn't really use those, we don't need to waste memory and resources on remote host. |
|
I would like to keep stopping status, but if you don't want see this state, we could add in the LSPClientFeatures a boolean getter which does the behaviour that you wish to do |
|
Or we could provide a customization of those Label getter of the server status to display. |
startStopTimer() was setting ServerStatus.stopping immediately when the lastDocumentDisconnectedTimeout timer started, not when the server actually began shutting down. This caused the LSP console to display "stopping pid:..." with a spinner for the entire grace period, even though the server was still fully functional and accepting connections. With the default 5-second timeout this was barely noticeable, but with longer timeouts (e.g. 180s for expensive-to-start servers) the UI was misleading — users could think the server was broken or unresponsive. Move updateStatus(ServerStatus.stopping) inside the alarm callback, just before stop(). The server now stays in 'started' status during the grace period and only transitions to 'stopping' when shutdown actually begins.
FIX=#1553