diff --git a/internal/lsp/client.go b/internal/lsp/client.go index 822916a..4b972ec 100644 --- a/internal/lsp/client.go +++ b/internal/lsp/client.go @@ -304,7 +304,12 @@ func (c *Client) send(msg interface{}) error { // On exit (for any reason), it drains all pending Call waiters with a // synthetic error so that goroutines blocked in Call are unblocked. func (c *Client) readLoop() { + // On exit (EOF/error from a crashed or closed server) mark the client not + // running so subsequent Calls fail fast — otherwise they pass the running + // guard, send to a dead stdin, and block until their own timeout. Then + // unblock any already-in-flight waiters. LIFO: setRunning(false) runs first. defer c.drainPending() + defer c.setRunning(false) reader := bufio.NewReader(c.stdout) @@ -380,13 +385,19 @@ func (c *Client) drainPending() { defer c.mu.Unlock() for id, ch := range c.pending { - ch <- &Response{ + // Non-blocking, matching the dispatch path: the channel is buffered(1) + // and read once, but a duplicate-id race could have already filled it — + // never block here while holding c.mu. + select { + case ch <- &Response{ JSONRPC: "2.0", ID: id, Error: &ResponseError{ Code: -32603, // InternalError Message: "LSP client readLoop terminated", }, + }: + default: } delete(c.pending, id) } @@ -398,3 +409,10 @@ func (c *Client) IsRunning() bool { defer c.runningMu.RUnlock() return c.running } + +// setRunning sets the running flag under its lock. +func (c *Client) setRunning(v bool) { + c.runningMu.Lock() + c.running = v + c.runningMu.Unlock() +} diff --git a/internal/lsp/client_test.go b/internal/lsp/client_test.go index f5b8a1e..929807a 100644 --- a/internal/lsp/client_test.go +++ b/internal/lsp/client_test.go @@ -1,10 +1,48 @@ package lsp import ( + "context" + "os/exec" "strings" "testing" + "time" ) +// TestClientMarksNotRunningOnProcessExit verifies crash detection: when the +// underlying server process exits (stdout EOF), readLoop must flip the client to +// not-running so later Calls fail fast instead of blocking until their timeout. +func TestClientMarksNotRunningOnProcessExit(t *testing.T) { + cmd := exec.Command("sh", "-c", "exit 0") // exits immediately, closing stdout + c, err := NewClient(cmd) + if err != nil { + t.Skipf("cannot start helper process: %v", err) + } + t.Cleanup(func() { _ = c.Close() }) + + deadline := time.Now().Add(2 * time.Second) + for c.IsRunning() && time.Now().Before(deadline) { + time.Sleep(10 * time.Millisecond) + } + if c.IsRunning() { + t.Fatal("client should be marked not running after its process exits") + } + + // A Call on the dead client must fail fast (the running guard), not hang. + done := make(chan error, 1) + go func() { + _, callErr := c.Call(context.Background(), "test/method", nil) + done <- callErr + }() + select { + case callErr := <-done: + if callErr == nil { + t.Error("Call on a dead client should return an error") + } + case <-time.After(2 * time.Second): + t.Error("Call on a dead client hung instead of failing fast") + } +} + // drainStderr retains a bounded tail of server stderr for diagnostics; capture // must keep recent bytes and never grow past StderrCaptureMax. func TestClientStderrCapture(t *testing.T) { diff --git a/internal/lsp/manager.go b/internal/lsp/manager.go index 4ded9d9..3da8ebd 100644 --- a/internal/lsp/manager.go +++ b/internal/lsp/manager.go @@ -128,7 +128,11 @@ func (m *Manager) GetServer(ctx context.Context, lang protocol.Language) (*Manag srv, exists := m.servers[lang] m.mu.RUnlock() - if exists && srv.ready { + // A cached server is only reusable if it is both initialized and still + // running — a crashed language server stays in the map with ready==true, so + // without the liveness check every later call would be routed to a dead + // process and block until timeout. + if exists && srv.ready && srv.client.IsRunning() { // Update lastUsed with server's own lock to avoid race condition srv.mu.Lock() srv.lastUsed = time.Now() @@ -141,13 +145,21 @@ func (m *Manager) GetServer(ctx context.Context, lang protocol.Language) (*Manag defer m.mu.Unlock() // Double-check after acquiring write lock - if srv, ok := m.servers[lang]; ok && srv.ready { + if srv, ok := m.servers[lang]; ok && srv.ready && srv.client.IsRunning() { srv.mu.Lock() srv.lastUsed = time.Now() srv.mu.Unlock() return srv, nil } + // Evict a stale entry (crashed or never-ready) before spawning a replacement. + // Its process has already exited, so close it asynchronously to avoid holding + // m.mu on the (fast) cmd.Wait(). + if stale, ok := m.servers[lang]; ok { + delete(m.servers, lang) + go func() { _ = stale.client.Close() }() + } + // Check if server config exists config, ok := DefaultServerConfigs[lang] if !ok {