internal/lsp/lsprpc: automatically resolve and start the remote gopls

Most users will not want to manage their own gopls instance, but may
still want to benefit from using a shared instance.

This CL adds support for an 'auto' network type that can be encoded in
the -remote flag similarly to UDS (i.e. -remote="auto;uniqueid"). In
this mode, the actual remote address will be resolved automatically
based on the executing environment and unique identifier, and the remote
server will be started if it isn't already running.

Updates golang/go#34111

Change-Id: Ib62159765a108f3645f57709b8ff079b39dd6727
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220137
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>

internal/lsp/cmd/cmd.go

@@ -57,7 +57,7 @@ type Application struct {
 	env []string
 
 	// Support for remote lsp server
-	Remote string `flag:"remote" help:"*EXPERIMENTAL* - forward all commands to a remote lsp specified by this flag. If prefixed by 'unix;', the subsequent address is assumed to be a unix domain socket. Otherwise, TCP is used."`
+	Remote string `flag:"remote" help:"*EXPERIMENTAL* - forward all commands to a remote lsp specified by this flag. If prefixed by 'unix;', the subsequent address is assumed to be a unix domain socket. If 'auto', or prefixed by 'auto', the remote address is automatically resolved based on the executing environment. Otherwise, TCP is used."`
 
 	// Enable verbose logging
 	Verbose bool `flag:"v" help:"verbose output"`

internal/lsp/cmd/serve.go

@@ -88,6 +88,11 @@ func (s *Serve) Run(ctx context.Context, args ...string) error {
 
 // parseAddr parses the -listen flag in to a network, and address.
 func parseAddr(listen string) (network string, address string) {
+	// Allow passing just -remote=auto, as a shorthand for using automatic remote
+	// resolution.
+	if listen == lsprpc.AutoNetwork {
+		return lsprpc.AutoNetwork, ""
+	}
 	if parts := strings.SplitN(listen, ";", 2); len(parts) == 2 {
 		return parts[0], parts[1]
 	}

internal/lsp/cmd/serve_test.go

@@ -12,6 +12,8 @@ func TestListenParsing(t *testing.T) {
 	}{
 		{"127.0.0.1:0", "tcp", "127.0.0.1:0"},
 		{"unix;/tmp/sock", "unix", "/tmp/sock"},
+		{"auto", "auto", ""},
+		{"auto;foo", "auto", "foo"},
 	}
 
 	for _, test := range tests {

internal/lsp/debug/serve.go

@@ -567,7 +567,26 @@ Unknown page
 `)).Funcs(template.FuncMap{
 	"fuint64": fuint64,
 	"fuint32": fuint32,
-	"url":     func(s string) template.URL { return template.URL(s) },
+	"localAddress": func(s string) string {
+		// Try to translate loopback addresses to localhost, both for cosmetics and
+		// because unspecified ipv6 addresses can break links on Windows.
+		//
+		// TODO(rfindley): In the future, it would be better not to assume the
+		// server is running on localhost, and instead construct this address using
+		// the remote host.
+		host, port, err := net.SplitHostPort(s)
+		if err != nil {
+			return s
+		}
+		ip := net.ParseIP(host)
+		if ip == nil {
+			return s
+		}
+		if ip.IsLoopback() || ip.IsUnspecified() {
+			return "localhost:" + port
+		}
+		return s
+	},
 })
 
 var mainTmpl = template.Must(template.Must(baseTemplate.Clone()).Parse(`
@@ -644,16 +663,18 @@ var clientTmpl = template.Must(template.Must(baseTemplate.Clone()).Parse(`
 {{define "title"}}Client {{.ID}}{{end}}
 {{define "body"}}
 Using session: <b>{{template "sessionlink" .Session.ID}}</b><br>
-Debug this client at: <a href="http://{{url .DebugAddress}}">{{.DebugAddress}}</a><br>
+Debug this client at: <a href="http://{{localAddress .DebugAddress}}">{{localAddress .DebugAddress}}</a><br>
 Logfile: {{.Logfile}}<br>
+Gopls Path: {{.GoplsPath}}<br>
 {{end}}
 `))
 
 var serverTmpl = template.Must(template.Must(baseTemplate.Clone()).Parse(`
 {{define "title"}}Server {{.ID}}{{end}}
 {{define "body"}}
-Debug this server at: <a href="http://{{.DebugAddress}}">{{.DebugAddress}}</a><br>
+Debug this server at: <a href="http://{{localAddress .DebugAddress}}">{{localAddress .DebugAddress}}</a><br>
 Logfile: {{.Logfile}}<br>
+Gopls Path: {{.GoplsPath}}<br>
 {{end}}
 `))
 

internal/lsp/lsprpc/autostart_posix.go

@@ -0,0 +1,49 @@
+// Copyright 2020 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// +build !windows
+
+package lsprpc
+
+import (
+	"crypto/sha1"
+	"fmt"
+	"log"
+	"os"
+	"os/exec"
+	"path/filepath"
+)
+
+// autoNetworkAddress resolves an id on the 'auto' pseduo-network to a
+// real network and address. On unix, this uses unix domain sockets.
+func autoNetworkAddress(goplsPath, id string) (network string, address string) {
+	// Especially when doing local development or testing, it's important that
+	// the remote gopls instance we connect to is running the same binary as our
+	// forwarder. So we encode a short hash of the binary path into the daemon
+	// socket name. If possible, we also include the buildid in this hash, to
+	// account for long-running processes where the binary has been subsequently
+	// rebuilt.
+	h := sha1.New()
+	cmd := exec.Command("go", "tool", "buildid", goplsPath)
+	cmd.Stdout = h
+	var pathHash []byte
+	if err := cmd.Run(); err == nil {
+		pathHash = h.Sum(nil)
+	} else {
+		log.Printf("error getting current buildid: %v", err)
+		sum := sha1.Sum([]byte(goplsPath))
+		pathHash = sum[:]
+	}
+	shortHash := fmt.Sprintf("%x", pathHash)[:6]
+	user := os.Getenv("USER")
+	if user == "" {
+		user = "shared"
+	}
+	basename := filepath.Base(goplsPath)
+	idComponent := ""
+	if id != "" {
+		idComponent = "-" + id
+	}
+	return "unix", filepath.Join(os.TempDir(), fmt.Sprintf("%s-%s-daemon.%s%s", basename, shortHash, user, idComponent))
+}

internal/lsp/lsprpc/autostart_windows.go

@@ -0,0 +1,17 @@
+// Copyright 2020 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// +build windows
+
+package lsprpc
+
+// autoNetworkAddress returns the default network and address for the
+// automatically-started gopls remote. See autostart_posix.go for more
+// information.
+func autoNetworkAddress(goplsPath, id string) (network string, address string) {
+	if id != "" {
+		panic("identified remotes are not supported on windows")
+	}
+	return "tcp", ":37374"
+}

internal/lsp/lsprpc/lsprpc.go

@@ -10,8 +10,10 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	stdlog "log"
 	"net"
 	"os"
+	"os/exec"
 	"strconv"
 	"sync/atomic"
 	"time"
@@ -25,6 +27,10 @@ import (
 	"golang.org/x/tools/internal/telemetry/log"
 )
 
+// AutoNetwork is the pseudo network type used to signal that gopls should use
+// automatic discovery to resolve a remote address.
+const AutoNetwork = "auto"
+
 // The StreamServer type is a jsonrpc2.StreamServer that handles incoming
 // streams as a new LSP session, using a shared cache.
 type StreamServer struct {
@@ -56,6 +62,7 @@ type debugInstance struct {
 	id           string
 	debugAddress string
 	logfile      string
+	goplsPath    string
 }
 
 func (d debugInstance) ID() string {
@@ -70,6 +77,10 @@ func (d debugInstance) Logfile() string {
 	return d.logfile
 }
 
+func (d debugInstance) GoplsPath() string {
+	return d.goplsPath
+}
+
 // A debugServer is held by the client to identity the remove server to which
 // it is connected.
 type debugServer struct {
@@ -124,9 +135,15 @@ func (s *StreamServer) ServeStream(ctx context.Context, stream jsonrpc2.Stream)
 	if s.withTelemetry {
 		conn.AddHandler(telemetryHandler{})
 	}
+	executable, err := os.Executable()
+	if err != nil {
+		stdlog.Printf("error getting gopls path: %v", err)
+		executable = ""
+	}
 	conn.AddHandler(&handshaker{
 		client:    dc,
 		debug:     s.debug,
+		goplsPath: executable,
 	})
 	return conn.Run(protocol.WithClient(ctx, client))
 }
@@ -146,11 +163,17 @@ type Forwarder struct {
 	dialTimeout   time.Duration
 	retries       int
 	debug         *debug.Instance
+	goplsPath     string
 }
 
 // NewForwarder creates a new Forwarder, ready to forward connections to the
 // remote server specified by network and addr.
 func NewForwarder(network, addr string, withTelemetry bool, debugInstance *debug.Instance) *Forwarder {
+	gp, err := os.Executable()
+	if err != nil {
+		stdlog.Printf("error getting gopls path for forwarder: %v", err)
+		gp = ""
+	}
 	return &Forwarder{
 		network:       network,
 		addr:          addr,
@@ -158,6 +181,7 @@ func NewForwarder(network, addr string, withTelemetry bool, debugInstance *debug
 		dialTimeout:   1 * time.Second,
 		retries:       5,
 		debug:         debugInstance,
+		goplsPath:     gp,
 	}
 }
 
@@ -167,28 +191,9 @@ func (f *Forwarder) ServeStream(ctx context.Context, stream jsonrpc2.Stream) err
 	clientConn := jsonrpc2.NewConn(stream)
 	client := protocol.ClientDispatcher(clientConn)
 
-	var (
-		netConn net.Conn
-		err     error
-	)
-	// Sometimes the forwarder will be started immediately after the server is
-	// started. To account for these cases, add in some simple retrying.
-	// Note that the number of total attempts is f.retries + 1.
-	for attempt := 0; attempt <= f.retries; attempt++ {
-		startDial := time.Now()
-		netConn, err = net.DialTimeout(f.network, f.addr, f.dialTimeout)
-		if err == nil {
-			break
-		}
-		log.Print(ctx, fmt.Sprintf("failed an attempt to connect to remote: %v\n", err))
-		// In case our failure was a fast-failure, ensure we wait at least
-		// f.dialTimeout before trying again.
-		if attempt != f.retries {
-			time.Sleep(f.dialTimeout - time.Since(startDial))
-		}
-	}
+	netConn, err := f.connectToRemote(ctx)
 	if err != nil {
-		return fmt.Errorf("forwarder: dialing remote: %v", err)
+		return fmt.Errorf("forwarder: connecting to remote: %v", err)
 	}
 	serverConn := jsonrpc2.NewConn(jsonrpc2.NewHeaderStream(netConn, netConn))
 	server := protocol.ServerDispatcher(serverConn)
@@ -217,24 +222,97 @@ func (f *Forwarder) ServeStream(ctx context.Context, stream jsonrpc2.Stream) err
 		hreq = handshakeRequest{
 			ServerID:  serverID,
 			Logfile:   f.debug.Logfile,
-			DebugAddr: f.debug.DebugAddress,
+			DebugAddr: f.debug.ListenedDebugAddress,
+			GoplsPath: f.goplsPath,
 		}
 		hresp handshakeResponse
 	)
 	if err := serverConn.Call(ctx, handshakeMethod, hreq, &hresp); err != nil {
-		log.Error(ctx, "gopls handshake failed", err)
+		log.Error(ctx, "forwarder: gopls handshake failed", err)
+	}
+	if hresp.GoplsPath != f.goplsPath {
+		log.Error(ctx, "", fmt.Errorf("forwarder: gopls path mismatch: forwarder is %q, remote is %q", f.goplsPath, hresp.GoplsPath))
 	}
 	f.debug.State.AddServer(debugServer{
 		debugInstance: debugInstance{
 			id:           serverID,
 			logfile:      hresp.Logfile,
 			debugAddress: hresp.DebugAddr,
+			goplsPath:    hresp.GoplsPath,
 		},
 		clientID: hresp.ClientID,
 	})
 	return g.Wait()
 }
 
+func (f *Forwarder) connectToRemote(ctx context.Context) (net.Conn, error) {
+	var (
+		netConn          net.Conn
+		err              error
+		network, address = f.network, f.addr
+	)
+	if f.network == AutoNetwork {
+		// f.network is overloaded to support a concept of 'automatic' addresses,
+		// which signals that the gopls remote address should be automatically
+		// derived.
+		// So we need to resolve a real network and address here.
+		network, address = autoNetworkAddress(f.goplsPath, f.addr)
+	}
+	// Try dialing our remote once, in case it is already running.
+	netConn, err = net.DialTimeout(network, address, f.dialTimeout)
+	if err == nil {
+		return netConn, nil
+	}
+	// If our remote is on the 'auto' network, start it if it doesn't exist.
+	if f.network == AutoNetwork {
+		if f.goplsPath == "" {
+			return nil, fmt.Errorf("cannot auto-start remote: gopls path is unknown")
+		}
+		if network == "unix" {
+			// Sometimes the socketfile isn't properly cleaned up when gopls shuts
+			// down. Since we have already tried and failed to dial this address, it
+			// should *usually* be safe to remove the socket before binding to the
+			// address.
+			// TODO(rfindley): there is probably a race here if multiple gopls
+			// instances are simultaneously starting up.
+			if _, err := os.Stat(address); err == nil {
+				if err := os.Remove(address); err != nil {
+					return nil, fmt.Errorf("removing remote socket file: %v", err)
+				}
+			}
+		}
+		if err := startRemote(f.goplsPath, network, address); err != nil {
+			return nil, fmt.Errorf("startRemote(%q, %q): %v", network, address, err)
+		}
+	}
+
+	// It can take some time for the newly started server to bind to our address,
+	// so we retry for a bit.
+	for retry := 0; retry < f.retries; retry++ {
+		startDial := time.Now()
+		netConn, err = net.DialTimeout(network, address, f.dialTimeout)
+		if err == nil {
+			return netConn, nil
+		}
+		log.Print(ctx, fmt.Sprintf("failed attempt #%d to connect to remote: %v\n", retry+2, err))
+		// In case our failure was a fast-failure, ensure we wait at least
+		// f.dialTimeout before trying again.
+		if retry != f.retries-1 {
+			time.Sleep(f.dialTimeout - time.Since(startDial))
+		}
+	}
+	return nil, fmt.Errorf("dialing remote: %v", err)
+}
+
+func startRemote(goplsPath, network, address string) error {
+	args := []string{"serve", "-listen", fmt.Sprintf(`%s;%s`, network, address), "-debug", ":0", "-logfile", "auto"}
+	cmd := exec.Command(goplsPath, args...)
+	if err := cmd.Start(); err != nil {
+		return fmt.Errorf("starting remote gopls: %v", err)
+	}
+	return nil
+}
+
 // ForwarderExitFunc is used to exit the forwarder process. It is mutable for
 // testing purposes.
 var ForwarderExitFunc = os.Exit
@@ -283,12 +361,14 @@ type handshaker struct {
 	jsonrpc2.EmptyHandler
 	client    *debugClient
 	debug     *debug.Instance
+	goplsPath string
 }
 
 type handshakeRequest struct {
 	ServerID  string `json:"serverID"`
 	Logfile   string `json:"logfile"`
 	DebugAddr string `json:"debugAddr"`
+	GoplsPath string `json:"goplsPath"`
 }
 
 type handshakeResponse struct {
@@ -296,6 +376,7 @@ type handshakeResponse struct {
 	SessionID string `json:"sessionID"`
 	Logfile   string `json:"logfile"`
 	DebugAddr string `json:"debugAddr"`
+	GoplsPath string `json:"goplsPath"`
 }
 
 const handshakeMethod = "gopls/handshake"
@@ -310,11 +391,13 @@ func (h *handshaker) Deliver(ctx context.Context, r *jsonrpc2.Request, delivered
 		h.client.debugAddress = req.DebugAddr
 		h.client.logfile = req.Logfile
 		h.client.serverID = req.ServerID
+		h.client.goplsPath = req.GoplsPath
 		resp := handshakeResponse{
 			ClientID:  h.client.id,
 			SessionID: cache.DebugSession{Session: h.client.session}.ID(),
 			Logfile:   h.debug.Logfile,
-			DebugAddr: h.debug.DebugAddress,
+			DebugAddr: h.debug.ListenedDebugAddress,
+			GoplsPath: h.goplsPath,
 		}
 		if err := r.Reply(ctx, resp, nil); err != nil {
 			log.Error(ctx, "replying to handshake", err)

internal/lsp/regtest/env.go

@@ -211,6 +211,8 @@ func (r *Runner) forwardedEnv(ctx context.Context, t *testing.T) (servertest.Con
 
 func (r *Runner) separateProcessEnv(ctx context.Context, t *testing.T) (servertest.Connector, func()) {
 	socket := r.getRemoteSocket(t)
+	// TODO(rfindley): can we use the autostart behavior here, instead of
+	// pre-starting the remote?
 	forwarder := lsprpc.NewForwarder("unix", socket, false, debug.NewInstance("", ""))
 	ts2 := servertest.NewPipeServer(ctx, forwarder)
 	cleanup := func() {