internal/lsp: add a command to start the debug server

The utility of the debug server is limited by the requirement to start
gopls with the `-debug` flag and then look in the logs to see which port
the debug server is bound to.

This CL adds a new custom command `gopls.startDebugging` to start the
debug server on demand if it isn't already running, and return its debug
address. It also does some gymnastics to make this turn on debugging for
any intermediate gopls forwarders, when using daemon mode.

For golang/go#45518

Change-Id: I48a90088f96aca54f34f93bedbfe864515320f61
Reviewed-on: https://go-review.googlesource.com/c/tools/+/309409
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>

2 files changed, 51 insertions, 22 deletions

diff --git a/internal/lsp/debug/info.go b/internal/lsp/debug/info.go
index d4580190a..3533b364e 100644
--- a/internal/lsp/debug/info.go
+++ b/internal/lsp/debug/info.go
@@ -100,7 +100,7 @@ func (i *Instance) PrintServerInfo(ctx context.Context, w io.Writer) {
 		fmt.Fprintf(w, "LogFile: %s\n", i.Logfile)
 		fmt.Fprintf(w, "Working directory: %s\n", i.Workdir)
 		fmt.Fprintf(w, "Address: %s\n", i.ServerAddress)
-		fmt.Fprintf(w, "Debug address: %s\n", i.DebugAddress)
+		fmt.Fprintf(w, "Debug address: %s\n", i.DebugAddress())
 	})
 	PrintVersionInfo(ctx, w, true, HTML)
 	section(w, HTML, "Command Line", func() {
diff --git a/internal/lsp/debug/serve.go b/internal/lsp/debug/serve.go
index 473518e43..9d55b2806 100644
--- a/internal/lsp/debug/serve.go
+++ b/internal/lsp/debug/serve.go
@@ -50,13 +50,11 @@ const (
 
 // An Instance holds all debug information associated with a gopls instance.
 type Instance struct {
-	Logfile              string
-	StartTime            time.Time
-	ServerAddress        string
-	DebugAddress         string
-	ListenedDebugAddress string
-	Workdir              string
-	OCAgentConfig        string
+	Logfile       string
+	StartTime     time.Time
+	ServerAddress string
+	Workdir       string
+	OCAgentConfig string
 
 	LogWriter io.Writer
 
@@ -67,6 +65,10 @@ type Instance struct {
 	rpcs       *Rpcs
 	traces     *traces
 	State      *State
+
+	serveMu              sync.Mutex
+	debugAddress         string
+	listenedDebugAddress string
 }
 
 // State holds debugging information related to the server state.
@@ -216,9 +218,15 @@ func (st *State) dropClient(session source.Session) {
 
 // AddServer adds a server to the set being queried. In practice, there should
 // be at most one remote server.
-func (st *State) addServer(server *Server) {
+func (st *State) updateServer(server *Server) {
 	st.mu.Lock()
 	defer st.mu.Unlock()
+	for _, existing := range st.servers {
+		if existing.ID == server.ID {
+			*existing = *server
+			return
+		}
+	}
 	st.servers = append(st.servers, server)
 }
 
@@ -274,11 +282,11 @@ func (i *Instance) getSession(r *http.Request) interface{} {
 	return i.State.Session(path.Base(r.URL.Path))
 }
 
-func (i Instance) getClient(r *http.Request) interface{} {
+func (i *Instance) getClient(r *http.Request) interface{} {
 	return i.State.Client(path.Base(r.URL.Path))
 }
 
-func (i Instance) getServer(r *http.Request) interface{} {
+func (i *Instance) getServer(r *http.Request) interface{} {
 	i.State.mu.Lock()
 	defer i.State.mu.Unlock()
 	id := path.Base(r.URL.Path)
@@ -290,7 +298,7 @@ func (i Instance) getServer(r *http.Request) interface{} {
 	return nil
 }
 
-func (i Instance) getView(r *http.Request) interface{} {
+func (i *Instance) getView(r *http.Request) interface{} {
 	return i.State.View(path.Base(r.URL.Path))
 }
 
@@ -395,22 +403,31 @@ func (i *Instance) SetLogFile(logfile string, isDaemon bool) (func(), error) {
 	return closeLog, nil
 }
 
-// Serve starts and runs a debug server in the background.
+// Serve starts and runs a debug server in the background on the given addr.
 // It also logs the port the server starts on, to allow for :0 auto assigned
 // ports.
-func (i *Instance) Serve(ctx context.Context) error {
+func (i *Instance) Serve(ctx context.Context, addr string) (string, error) {
 	stdlog.SetFlags(stdlog.Lshortfile)
-	if i.DebugAddress == "" {
-		return nil
+	if addr == "" {
+		return "", nil
+	}
+	i.serveMu.Lock()
+	defer i.serveMu.Unlock()
+
+	if i.listenedDebugAddress != "" {
+		// Already serving. Return the bound address.
+		return i.listenedDebugAddress, nil
 	}
-	listener, err := net.Listen("tcp", i.DebugAddress)
+
+	i.debugAddress = addr
+	listener, err := net.Listen("tcp", i.debugAddress)
 	if err != nil {
-		return err
+		return "", err
 	}
-	i.ListenedDebugAddress = listener.Addr().String()
+	i.listenedDebugAddress = listener.Addr().String()
 
 	port := listener.Addr().(*net.TCPAddr).Port
-	if strings.HasSuffix(i.DebugAddress, ":0") {
+	if strings.HasSuffix(i.debugAddress, ":0") {
 		stdlog.Printf("debug server listening at http://localhost:%d", port)
 	}
 	event.Log(ctx, "Debug serving", tag.Port.Of(port))
@@ -446,7 +463,19 @@ func (i *Instance) Serve(ctx context.Context) error {
 		}
 		event.Log(ctx, "Debug server finished")
 	}()
-	return nil
+	return i.listenedDebugAddress, nil
+}
+
+func (i *Instance) DebugAddress() string {
+	i.serveMu.Lock()
+	defer i.serveMu.Unlock()
+	return i.debugAddress
+}
+
+func (i *Instance) ListenedDebugAddress() string {
+	i.serveMu.Lock()
+	defer i.serveMu.Unlock()
+	return i.listenedDebugAddress
 }
 
 // MonitorMemory starts recording memory statistics each second.
@@ -579,7 +608,7 @@ func makeInstanceExporter(i *Instance) event.Exporter {
 				i.State.addClient(s)
 			}
 			if sid := tag.NewServer.Get(ev); sid != "" {
-				i.State.addServer(&Server{
+				i.State.updateServer(&Server{
 					ID:           sid,
 					Logfile:      tag.Logfile.Get(ev),
 					DebugAddress: tag.DebugAddress.Get(ev),