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 <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>

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

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