From c1c69b635fc595c959586ff3b11710120d5bae23 Mon Sep 17 00:00:00 2001 From: Rob Findley Date: Mon, 9 Mar 2020 14:17:49 -0400 Subject: [PATCH] internal/lsp: clean up on Shutdown even if not initialized Previously it was an error to call shutdown while the server was not yet initialized. This can result in session leak for very short-lived sessions. Instead, allow shutdown to proceed and simply log the error. On the other hand, for consistency make it an error to call initialized without first calling initialize. Updates golang/go#34111 Change-Id: I0330d81323ddda6beec0f6ed9eb065f8b717dea0 Reviewed-on: https://go-review.googlesource.com/c/tools/+/222671 Run-TryBot: Robert Findley TryBot-Result: Gobot Gobot Reviewed-by: Rebecca Stambler Reviewed-by: Heschi Kreinick --- internal/lsp/general.go | 14 ++++++++------ internal/lsp/server.go | 15 +++++++++++++++ 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/internal/lsp/general.go b/internal/lsp/general.go index a4cbf42496..f2b735dcdc 100644 --- a/internal/lsp/general.go +++ b/internal/lsp/general.go @@ -22,12 +22,10 @@ import ( func (s *Server) initialize(ctx context.Context, params *protocol.ParamInitialize) (*protocol.InitializeResult, error) { s.stateMu.Lock() - state := s.state - s.stateMu.Unlock() - if state >= serverInitializing { - return nil, jsonrpc2.NewErrorf(jsonrpc2.CodeInvalidRequest, "server already initialized") + if s.state >= serverInitializing { + defer s.stateMu.Unlock() + return nil, jsonrpc2.NewErrorf(jsonrpc2.CodeInvalidRequest, "initialize called while server in %v state", s.state) } - s.stateMu.Lock() s.state = serverInitializing s.stateMu.Unlock() @@ -111,6 +109,10 @@ func (s *Server) initialize(ctx context.Context, params *protocol.ParamInitializ func (s *Server) initialized(ctx context.Context, params *protocol.InitializedParams) error { s.stateMu.Lock() + if s.state >= serverInitialized { + defer s.stateMu.Unlock() + return jsonrpc2.NewErrorf(jsonrpc2.CodeInvalidRequest, "initalized called while server in %v state", s.state) + } s.state = serverInitialized s.stateMu.Unlock() @@ -264,7 +266,7 @@ func (s *Server) shutdown(ctx context.Context) error { s.stateMu.Lock() defer s.stateMu.Unlock() if s.state < serverInitialized { - return jsonrpc2.NewErrorf(jsonrpc2.CodeInvalidRequest, "server not initialized") + event.Print(ctx, "server shutdown without initialization") } if s.state != serverShutDown { // drop all the active views diff --git a/internal/lsp/server.go b/internal/lsp/server.go index 65c27e0ccb..010e313c1b 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -7,6 +7,7 @@ package lsp import ( "context" + "fmt" "sync" "golang.org/x/tools/internal/jsonrpc2" @@ -38,6 +39,20 @@ const ( serverShutDown ) +func (s serverState) String() string { + switch s { + case serverCreated: + return "created" + case serverInitializing: + return "initializing" + case serverInitialized: + return "initialized" + case serverShutDown: + return "shutDown" + } + return fmt.Sprintf("(unknown state: %d)", int(s)) +} + // Server implements the protocol.Server interface. type Server struct { client protocol.Client