git.iko.soy/crush
on main help sign in
fix(lsp): fallback to Kill() on timeout (#2349)
56d79d1Tai Groot
internal/lsp/client.go+25−6
compare with main
unifiedsplit
@@ -125,19 +125,38 @@ func (c *Client) Initialize(ctx context.Context, workspaceDir string) (*protocol
125 return result, nil
125 return result, nil
126}
126}
127
127
128+// closeTimeout is the maximum time to wait for a graceful LSP shutdown.
129+const closeTimeout = 5 * time.Second
130+
128// Kill kills the client without doing anything else.
131// Kill kills the client without doing anything else.
129func (c *Client) Kill() { c.client.Kill() }
132func (c *Client) Kill() { c.client.Kill() }
130
133
131−// Close closes all open files in the client, then the client.
134+// Close closes all open files in the client, then shuts down gracefully.
135+// If shutdown takes longer than closeTimeout, it falls back to Kill().
132func (c *Client) Close(ctx context.Context) error {
136func (c *Client) Close(ctx context.Context) error {
133 c.CloseAllFiles(ctx)
137 c.CloseAllFiles(ctx)
134
138
135− // Shutdown and exit the client
139+ // Use a timeout to prevent hanging on unresponsive LSP servers.
136− if err := c.client.Shutdown(ctx); err != nil {
140+ // jsonrpc2's send lock doesn't respect context cancellation, so we
137− slog.Warn("Failed to shutdown LSP client", "error", err)
141+ // need to fall back to Kill() which closes the underlying connection.
138− }
142+ closeCtx, cancel := context.WithTimeout(ctx, closeTimeout)
143+ defer cancel()
139
144
145+ done := make(chan error, 1)
146+ go func() {
147+ if err := c.client.Shutdown(closeCtx); err != nil {
148+ slog.Warn("Failed to shutdown LSP client", "error", err)
149+ }
140− return c.client.Exit()
150+ done <- c.client.Exit()
151+ }()
152+
153+ select {
154+ case err := <-done:
155+ return err
156+ case <-closeCtx.Done():
157+ c.client.Kill()
158+ return closeCtx.Err()
159+ }
141}
160}
142
161
143// createPowernapClient creates a new powernap client with the current configuration.
162// createPowernapClient creates a new powernap client with the current configuration.
internal/lsp/manager.go+2−2
@@ -205,7 +205,7 @@ func (s *Manager) startServer(ctx context.Context, name, filepath string, server
205 if existing, ok := s.clients.Get(name); ok {
205 if existing, ok := s.clients.Get(name); ok {
206 switch existing.GetServerState() {
206 switch existing.GetServerState() {
207 case StateReady, StateStarting, StateDisabled:
207 case StateReady, StateStarting, StateDisabled:
208− client.Close(ctx)
208+ _ = client.Close(ctx)
209 s.callback(name, existing)
209 s.callback(name, existing)
210 return
210 return
211 }
211 }
@@ -228,7 +228,7 @@ func (s *Manager) startServer(ctx context.Context, name, filepath string, server
228
228
229 if _, err := client.Initialize(initCtx, s.cfg.WorkingDir()); err != nil {
229 if _, err := client.Initialize(initCtx, s.cfg.WorkingDir()); err != nil {
230 slog.Error("LSP client initialization failed", "name", name, "error", err)
230 slog.Error("LSP client initialization failed", "name", name, "error", err)
231− client.Close(ctx)
231+ _ = client.Close(ctx)
232 s.clients.Del(name)
232 s.clients.Del(name)
233 return
233 return
234 }
234 }