ipn/ipnserver: make Engine argument a func that tries again for each connection

So a backend in server-an-error state (as used by Windows) can try to
create a new Engine again each time somebody re-connects, relaunching
the GUI app.

(The proper fix is actually fixing Windows issues, but this makes things better
in the short term)

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

cmd/tailscaled/tailscaled.go

@@ -169,7 +169,7 @@ func run() error {
 		SurviveDisconnects: true,
 		DebugMux:           debugMux,
 	}
-	err = ipnserver.Run(ctx, logf, pol.PublicID.String(), opts, e)
+	err = ipnserver.Run(ctx, logf, pol.PublicID.String(), ipnserver.FixedEngine(e), opts)
 	// Cancelation is not an error: it is the only way to stop ipnserver.
 	if err != nil && err != context.Canceled {
 		logf("ipnserver.Run: %v", err)

ipn/ipnserver/server.go

@@ -69,10 +69,6 @@ type Options struct {
 	// DebugMux, if non-nil, specifies an HTTP ServeMux in which
 	// to register a debug handler.
 	DebugMux *http.ServeMux
-
-	// ErrorMessage, if not empty, signals that the server will exist
-	// only to relay the provided critical error message to the user.
-	ErrorMessage string
 }
 
 // server is an IPN backend and its set of 0 or more active connections
@@ -152,7 +148,9 @@ func (s *server) writeToClients(b []byte) {
 	}
 }
 
-func Run(ctx context.Context, logf logger.Logf, logid string, opts Options, e wgengine.Engine) error {
+// Run runs a Tailscale backend service.
+// The getEngine func is called repeatedly, once per connection, until it returns an engine successfully.
+func Run(ctx context.Context, logf logger.Logf, logid string, getEngine func() (wgengine.Engine, error), opts Options) error {
 	runDone := make(chan struct{})
 	defer close(runDone)
 
@@ -179,7 +177,9 @@ func Run(ctx context.Context, logf logger.Logf, logid string, opts Options, e wg
 
 	bo := backoff.NewBackoff("ipnserver", logf)
 
-	if opts.ErrorMessage != "" {
+	eng, err := getEngine()
+	if err != nil {
+		logf("Initial getEngine call: %v", err)
 		for i := 1; ctx.Err() == nil; i++ {
 			s, err := listen.Accept()
 			if err != nil {
@@ -187,13 +187,20 @@ func Run(ctx context.Context, logf logger.Logf, logid string, opts Options, e wg
 				bo.BackOff(ctx, err)
 				continue
 			}
-			serverToClient := func(b []byte) {
+			logf("%d: trying getEngine again...", i)
-				ipn.WriteMsg(s, b)
+			//lint:ignore SA4006 staticcheck is wrong
+			eng, err = getEngine()
+			if err == nil {
+				logf("%d: GetEngine worked; exiting failure loop", i)
+				break
 			}
+			logf("%d: getEngine failed again: %v", i, err)
+			errMsg := err.Error()
 			go func() {
 				defer s.Close()
+				serverToClient := func(b []byte) { ipn.WriteMsg(s, b) }
 				bs := ipn.NewBackendServer(logf, nil, serverToClient)
-				bs.SendErrorMessage(opts.ErrorMessage)
+				bs.SendErrorMessage(errMsg)
 				s.Read(make([]byte, 1))
 			}()
 		}
@@ -210,7 +217,7 @@ func Run(ctx context.Context, logf logger.Logf, logid string, opts Options, e wg
 		store = &ipn.MemoryStore{}
 	}
 
-	b, err := ipn.NewLocalBackend(logf, logid, store, e)
+	b, err := ipn.NewLocalBackend(logf, logid, store, eng)
 	if err != nil {
 		return fmt.Errorf("NewLocalBackend: %v", err)
 	}
@@ -371,3 +378,8 @@ func BabysitProc(ctx context.Context, args []string, logf logger.Logf) {
 		}
 	}
 }
+
+// FixedEngine returns a func that returns eng and a nil error.
+func FixedEngine(eng wgengine.Engine) func() (wgengine.Engine, error) {
+	return func() (wgengine.Engine, error) { return eng, nil }
+}

ipn/ipnserver/server_test.go

@@ -72,6 +72,6 @@ func TestRunMultipleAccepts(t *testing.T) {
 		SocketPath: socketPath,
 	}
 	t.Logf("pre-Run")
-	err = ipnserver.Run(ctx, logTriggerTestf, "dummy_logid", opts, eng)
+	err = ipnserver.Run(ctx, logTriggerTestf, "dummy_logid", ipnserver.FixedEngine(eng), opts)
 	t.Logf("ipnserver.Run = %v", err)
 }