internal/lsp/lsprpc: add an AutoDialer abstraction

Refactor the lsprpc package to move the logic for 'automatic' server
discovery into an AutoDialer abstraction, which both implements the v2
jsonrpc2 Dialer interface, and provides a dialNet method that can be
used for the existing v1 APIs.

Along the way, simplify the evaluation of remote arguments to eliminate
the overly abstract RemoteOption.

Change-Id: Ic3def17ccc237007a7eb2cc41a12cf058fca9be3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/332490
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: Ian Cottrell <iancottrell@google.com>

internal/lsp/cmd/serve.go

@@ -56,6 +56,22 @@ gopls server flags are:
 	f.PrintDefaults()
 }
 
+func (s *Serve) remoteArgs(network, address string) []string {
+	args := []string{"serve",
+		"-listen", fmt.Sprintf(`%s;%s`, network, address),
+	}
+	if s.RemoteDebug != "" {
+		args = append(args, "-debug", s.RemoteDebug)
+	}
+	if s.RemoteListenTimeout != 0 {
+		args = append(args, "-listen.timeout", s.RemoteListenTimeout.String())
+	}
+	if s.RemoteLogfile != "" {
+		args = append(args, "-logfile", s.RemoteLogfile)
+	}
+	return args
+}
+
 // Run configures a server based on the flags, and then runs it.
 // It blocks until the server shuts down.
 func (s *Serve) Run(ctx context.Context, args ...string) error {
@@ -77,12 +93,11 @@ func (s *Serve) Run(ctx context.Context, args ...string) error {
 	}
 	var ss jsonrpc2.StreamServer
 	if s.app.Remote != "" {
-		network, addr := lsprpc.ParseAddr(s.app.Remote)
-		ss = lsprpc.NewForwarder(network, addr,
-			lsprpc.RemoteDebugAddress(s.RemoteDebug),
-			lsprpc.RemoteListenTimeout(s.RemoteListenTimeout),
-			lsprpc.RemoteLogfile(s.RemoteLogfile),
-		)
+		var err error
+		ss, err = lsprpc.NewForwarder(s.app.Remote, s.remoteArgs)
+		if err != nil {
+			return errors.Errorf("creating forwarder: %w", err)
+		}
 	} else {
 		ss = lsprpc.NewStreamServer(cache.New(s.app.options), isDaemon)
 	}

internal/lsp/lsprpc/autostart_default.go

@@ -11,13 +11,13 @@ import (
 )
 
 var (
-	startRemote           = startRemoteDefault
+	daemonize             = func(*exec.Cmd) {}
 	autoNetworkAddress    = autoNetworkAddressDefault
 	verifyRemoteOwnership = verifyRemoteOwnershipDefault
 )
 
-func startRemoteDefault(goplsPath string, args ...string) error {
-	cmd := exec.Command(goplsPath, args...)
+func runRemote(cmd *exec.Cmd) error {
+	daemonize(cmd)
 	if err := cmd.Start(); err != nil {
 		return errors.Errorf("starting remote gopls: %w", err)
 	}

internal/lsp/lsprpc/autostart_posix.go

@@ -11,32 +11,28 @@ import (
 	"crypto/sha256"
 	"errors"
 	"fmt"
-	exec "golang.org/x/sys/execabs"
 	"log"
 	"os"
 	"os/user"
 	"path/filepath"
 	"strconv"
 	"syscall"
 
+	exec "golang.org/x/sys/execabs"
+
 	"golang.org/x/xerrors"
 )
 
 func init() {
-	startRemote = startRemotePosix
+	daemonize = daemonizePosix
 	autoNetworkAddress = autoNetworkAddressPosix
 	verifyRemoteOwnership = verifyRemoteOwnershipPosix
 }
 
-func startRemotePosix(goplsPath string, args ...string) error {
-	cmd := exec.Command(goplsPath, args...)
+func daemonizePosix(cmd *exec.Cmd) {
 	cmd.SysProcAttr = &syscall.SysProcAttr{
 		Setsid: true,
 	}
-	if err := cmd.Start(); err != nil {
-		return xerrors.Errorf("starting remote gopls: %w", err)
-	}
-	return nil
 }
 
 // autoNetworkAddress resolves an id on the 'auto' pseduo-network to a

internal/lsp/lsprpc/dialer.go

@@ -0,0 +1,115 @@
+// Copyright 2021 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.
+
+package lsprpc
+
+import (
+	"context"
+	"fmt"
+	"io"
+	"net"
+	"os"
+	"time"
+
+	exec "golang.org/x/sys/execabs"
+	"golang.org/x/tools/internal/event"
+	errors "golang.org/x/xerrors"
+)
+
+// AutoNetwork is the pseudo network type used to signal that gopls should use
+// automatic discovery to resolve a remote address.
+const AutoNetwork = "auto"
+
+// An AutoDialer is a jsonrpc2 dialer that understands the 'auto' network.
+type AutoDialer struct {
+	network, addr string // the 'real' network and address
+	isAuto        bool   // whether the server is on the 'auto' network
+
+	executable string
+	argFunc    func(network, addr string) []string
+}
+
+func NewAutoDialer(rawAddr string, argFunc func(network, addr string) []string) (*AutoDialer, error) {
+	d := AutoDialer{
+		argFunc: argFunc,
+	}
+	d.network, d.addr = ParseAddr(rawAddr)
+	if d.network == AutoNetwork {
+		d.isAuto = true
+		bin, err := os.Executable()
+		if err != nil {
+			return nil, errors.Errorf("getting executable: %w", err)
+		}
+		d.executable = bin
+		d.network, d.addr = autoNetworkAddress(bin, d.addr)
+	}
+	return &d, nil
+}
+
+// Dial implements the jsonrpc2.Dialer interface.
+func (d *AutoDialer) Dial(ctx context.Context) (io.ReadWriteCloser, error) {
+	conn, err := d.dialNet(ctx)
+	return conn, err
+}
+
+// TODO(rFindley): remove this once we no longer need to integrate with v1 of
+// the jsonrpc2 package.
+func (d *AutoDialer) dialNet(ctx context.Context) (net.Conn, error) {
+	// Attempt to verify that we own the remote. This is imperfect, but if we can
+	// determine that the remote is owned by a different user, we should fail.
+	ok, err := verifyRemoteOwnership(d.network, d.addr)
+	if err != nil {
+		// If the ownership check itself failed, we fail open but log an error to
+		// the user.
+		event.Error(ctx, "unable to check daemon socket owner, failing open", err)
+	} else if !ok {
+		// We successfully checked that the socket is not owned by us, we fail
+		// closed.
+		return nil, fmt.Errorf("socket %q is owned by a different user", d.addr)
+	}
+	const dialTimeout = 1 * time.Second
+	// Try dialing our remote once, in case it is already running.
+	netConn, err := net.DialTimeout(d.network, d.addr, dialTimeout)
+	if err == nil {
+		return netConn, nil
+	}
+	if d.isAuto && d.argFunc != nil {
+		if d.network == "unix" {
+			// Sometimes the socketfile isn't properly cleaned up when the server
+			// 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 server
+			// instances are simultaneously starting up.
+			if _, err := os.Stat(d.addr); err == nil {
+				if err := os.Remove(d.addr); err != nil {
+					return nil, errors.Errorf("removing remote socket file: %w", err)
+				}
+			}
+		}
+		args := d.argFunc(d.network, d.addr)
+		cmd := exec.Command(d.executable, args...)
+		if err := runRemote(cmd); err != nil {
+			return nil, err
+		}
+	}
+
+	const retries = 5
+	// 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 < retries; retry++ {
+		startDial := time.Now()
+		netConn, err = net.DialTimeout(d.network, d.addr, dialTimeout)
+		if err == nil {
+			return netConn, nil
+		}
+		event.Log(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 != retries-1 {
+			time.Sleep(dialTimeout - time.Since(startDial))
+		}
+	}
+	return nil, errors.Errorf("dialing remote: %w", err)
+}