diff --git a/doc/concurrency-resource-audit.md b/doc/concurrency-resource-audit.md index 1f4b3b1..b7ef904 100644 --- a/doc/concurrency-resource-audit.md +++ b/doc/concurrency-resource-audit.md @@ -262,11 +262,9 @@ Cancellation removes the request from the active set. `NextPending` discards sta ### M10. LSP shutdown is not idempotent -**Evidence:** `tools/lsp/server.go:41-51,112-121`. +**Status:** Fixed in the working tree. -`Stop()` has no single-owner guard. Repeated or concurrent calls can race on stdin, `Wait()`, and the reader loop. Startup failure after pipe creation does not close all pipes. - -**Remediation:** use `sync.Once` or an explicit lifecycle state machine; close pipes on every startup failure. +`Stop` is guarded by `sync.Once`, closes the process pipes, reaps the child, and releases pending requests when shutdown completes. Startup failure closes every pipe acquired before `cmd.Start` fails. ### M11. LSP timed-out requests retain channels until late responses diff --git a/tools/lsp/server.go b/tools/lsp/server.go index 1a0a89c..7c32791 100644 --- a/tools/lsp/server.go +++ b/tools/lsp/server.go @@ -15,6 +15,11 @@ import ( "time" ) +type response struct { + result json.RawMessage + err error +} + // Server represents a running LSP server process. type Server struct { adapter *Adapter @@ -25,11 +30,12 @@ type Server struct { mu sync.Mutex reqID atomic.Int64 - pending map[int64]chan json.RawMessage + pending map[int64]chan response diags map[string]json.RawMessage // uri → diagnostics open map[string]bool - done chan struct{} + done chan struct{} + stopOnce sync.Once } // StartServer launches an LSP server and performs the initialize handshake. @@ -48,6 +54,8 @@ func StartServer(adapter *Adapter, root string) (*Server, error) { return nil, err } if err := cmd.Start(); err != nil { + stdin.Close() + stdout.Close() return nil, err } @@ -57,7 +65,7 @@ func StartServer(adapter *Adapter, root string) (*Server, error) { cmd: cmd, stdin: stdin, stdout: stdout, - pending: make(map[int64]chan json.RawMessage), + pending: make(map[int64]chan response), diags: make(map[string]json.RawMessage), open: make(map[string]bool), done: make(chan struct{}), @@ -109,15 +117,24 @@ func (s *Server) initialize() error { return nil } -// Stop shuts down the LSP server gracefully. +// Stop shuts down the LSP server gracefully. It is safe to call concurrently. func (s *Server) Stop() { - s.Request("shutdown", map[string]any{}, 5*time.Second) //nolint:errcheck - s.Notify("exit", nil) - s.stdin.Close() - timer := time.AfterFunc(5*time.Second, func() { s.cmd.Process.Kill() }) - s.cmd.Wait() - timer.Stop() - <-s.done + s.stopOnce.Do(func() { + s.Request("shutdown", map[string]any{}, 5*time.Second) //nolint:errcheck + s.Notify("exit", nil) + s.stdin.Close() + timer := time.AfterFunc(5*time.Second, func() { s.cmd.Process.Kill() }) + s.cmd.Wait() + timer.Stop() + s.stdout.Close() + s.mu.Lock() + for id, ch := range s.pending { + ch <- response{err: fmt.Errorf("LSP server stopped")} + delete(s.pending, id) + } + s.mu.Unlock() + <-s.done + }) } // EnsureOpen sends textDocument/didOpen if the file hasn't been opened yet. @@ -146,7 +163,7 @@ func (s *Server) EnsureOpen(file string) { // Request sends a JSON-RPC request and waits for the response. func (s *Server) Request(method string, params any, timeout time.Duration) (json.RawMessage, error) { id := s.reqID.Add(1) - ch := make(chan json.RawMessage, 1) + ch := make(chan response, 1) s.mu.Lock() s.pending[id] = ch @@ -166,8 +183,8 @@ func (s *Server) Request(method string, params any, timeout time.Duration) (json } select { - case result := <-ch: - return result, nil + case resp := <-ch: + return resp.result, resp.err case <-time.After(timeout): s.mu.Lock() delete(s.pending, id) @@ -263,9 +280,9 @@ func (s *Server) readLoop() { s.mu.Unlock() if ch != nil { if msg.Error != nil { - ch <- nil + ch <- response{err: fmt.Errorf("LSP request failed: %s", msg.Error.Message)} } else { - ch <- msg.Result + ch <- response{result: msg.Result} } } } else if msg.Method == "textDocument/publishDiagnostics" {