Make LSP shutdown idempotent
This commit is contained in:
parent
4b86ad94ff
commit
55d1814aa7
|
|
@ -262,11 +262,9 @@ Cancellation removes the request from the active set. `NextPending` discards sta
|
||||||
|
|
||||||
### M10. LSP shutdown is not idempotent
|
### 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.
|
`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.
|
||||||
|
|
||||||
**Remediation:** use `sync.Once` or an explicit lifecycle state machine; close pipes on every startup failure.
|
|
||||||
|
|
||||||
### M11. LSP timed-out requests retain channels until late responses
|
### M11. LSP timed-out requests retain channels until late responses
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -15,6 +15,11 @@ import (
|
||||||
"time"
|
"time"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
type response struct {
|
||||||
|
result json.RawMessage
|
||||||
|
err error
|
||||||
|
}
|
||||||
|
|
||||||
// Server represents a running LSP server process.
|
// Server represents a running LSP server process.
|
||||||
type Server struct {
|
type Server struct {
|
||||||
adapter *Adapter
|
adapter *Adapter
|
||||||
|
|
@ -25,11 +30,12 @@ type Server struct {
|
||||||
|
|
||||||
mu sync.Mutex
|
mu sync.Mutex
|
||||||
reqID atomic.Int64
|
reqID atomic.Int64
|
||||||
pending map[int64]chan json.RawMessage
|
pending map[int64]chan response
|
||||||
diags map[string]json.RawMessage // uri → diagnostics
|
diags map[string]json.RawMessage // uri → diagnostics
|
||||||
open map[string]bool
|
open map[string]bool
|
||||||
|
|
||||||
done chan struct{}
|
done chan struct{}
|
||||||
|
stopOnce sync.Once
|
||||||
}
|
}
|
||||||
|
|
||||||
// StartServer launches an LSP server and performs the initialize handshake.
|
// 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
|
return nil, err
|
||||||
}
|
}
|
||||||
if err := cmd.Start(); err != nil {
|
if err := cmd.Start(); err != nil {
|
||||||
|
stdin.Close()
|
||||||
|
stdout.Close()
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -57,7 +65,7 @@ func StartServer(adapter *Adapter, root string) (*Server, error) {
|
||||||
cmd: cmd,
|
cmd: cmd,
|
||||||
stdin: stdin,
|
stdin: stdin,
|
||||||
stdout: stdout,
|
stdout: stdout,
|
||||||
pending: make(map[int64]chan json.RawMessage),
|
pending: make(map[int64]chan response),
|
||||||
diags: make(map[string]json.RawMessage),
|
diags: make(map[string]json.RawMessage),
|
||||||
open: make(map[string]bool),
|
open: make(map[string]bool),
|
||||||
done: make(chan struct{}),
|
done: make(chan struct{}),
|
||||||
|
|
@ -109,15 +117,24 @@ func (s *Server) initialize() error {
|
||||||
return nil
|
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() {
|
func (s *Server) Stop() {
|
||||||
s.Request("shutdown", map[string]any{}, 5*time.Second) //nolint:errcheck
|
s.stopOnce.Do(func() {
|
||||||
s.Notify("exit", nil)
|
s.Request("shutdown", map[string]any{}, 5*time.Second) //nolint:errcheck
|
||||||
s.stdin.Close()
|
s.Notify("exit", nil)
|
||||||
timer := time.AfterFunc(5*time.Second, func() { s.cmd.Process.Kill() })
|
s.stdin.Close()
|
||||||
s.cmd.Wait()
|
timer := time.AfterFunc(5*time.Second, func() { s.cmd.Process.Kill() })
|
||||||
timer.Stop()
|
s.cmd.Wait()
|
||||||
<-s.done
|
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.
|
// 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.
|
// 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) {
|
func (s *Server) Request(method string, params any, timeout time.Duration) (json.RawMessage, error) {
|
||||||
id := s.reqID.Add(1)
|
id := s.reqID.Add(1)
|
||||||
ch := make(chan json.RawMessage, 1)
|
ch := make(chan response, 1)
|
||||||
|
|
||||||
s.mu.Lock()
|
s.mu.Lock()
|
||||||
s.pending[id] = ch
|
s.pending[id] = ch
|
||||||
|
|
@ -166,8 +183,8 @@ func (s *Server) Request(method string, params any, timeout time.Duration) (json
|
||||||
}
|
}
|
||||||
|
|
||||||
select {
|
select {
|
||||||
case result := <-ch:
|
case resp := <-ch:
|
||||||
return result, nil
|
return resp.result, resp.err
|
||||||
case <-time.After(timeout):
|
case <-time.After(timeout):
|
||||||
s.mu.Lock()
|
s.mu.Lock()
|
||||||
delete(s.pending, id)
|
delete(s.pending, id)
|
||||||
|
|
@ -263,9 +280,9 @@ func (s *Server) readLoop() {
|
||||||
s.mu.Unlock()
|
s.mu.Unlock()
|
||||||
if ch != nil {
|
if ch != nil {
|
||||||
if msg.Error != nil {
|
if msg.Error != nil {
|
||||||
ch <- nil
|
ch <- response{err: fmt.Errorf("LSP request failed: %s", msg.Error.Message)}
|
||||||
} else {
|
} else {
|
||||||
ch <- msg.Result
|
ch <- response{result: msg.Result}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
} else if msg.Method == "textDocument/publishDiagnostics" {
|
} else if msg.Method == "textDocument/publishDiagnostics" {
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue