From 7074a40c060f01fbe80f643ffbeb3e98197b38e1 Mon Sep 17 00:00:00 2001
From: Maisem Ali <maisem@tailscale.com>
Date: Thu, 31 Aug 2023 14:31:23 -0700
Subject: [PATCH] control/controlclient: run SetControlClientStatus in
 goroutine

We have cases where the SetControlClientStatus would result in
a Shutdown call back into the auto client that would block
forever. The right thing to do here is to fix the LocalBackend
state machine but thats a different dumpster fire that we
are slowly making progress towards.

This makes it so that the SetControlClientStatus happens in a
different goroutine so that calls back into the auto client
do not block.

Also add a few missing mu.Unlocks in LocalBackend.Start.

Updates #9181

Signed-off-by: Maisem Ali <maisem@tailscale.com>
---
 control/controlclient/auto.go | 12 ++++--------
 ipn/ipnlocal/local.go         |  1 +
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/control/controlclient/auto.go b/control/controlclient/auto.go
index f1c6ebb60..8fbc3c904 100644
--- a/control/controlclient/auto.go
+++ b/control/controlclient/auto.go
@@ -159,7 +159,6 @@ type Auto struct {
 	loggedIn       bool       // true if currently logged in
 	loginGoal      *LoginGoal // non-nil if some login activity is desired
 	synced         bool       // true if our netmap is up-to-date
-	inSendStatus   int        // number of sendStatus calls currently in progress
 	state          State
 
 	authCtx    context.Context // context used for auth requests
@@ -646,7 +645,6 @@ func (c *Auto) sendStatus(who string, err error, url string, nm *netmap.NetworkM
 	state := c.state
 	loggedIn := c.loggedIn
 	synced := c.synced
-	c.inSendStatus++
 	c.mu.Unlock()
 
 	c.logf("[v1] sendStatus: %s: %v", who, state)
@@ -666,11 +664,10 @@ func (c *Auto) sendStatus(who string, err error, url string, nm *netmap.NetworkM
 		Err:     err,
 		state:   state,
 	}
-	c.observer.SetControlClientStatus(new)
 
-	c.mu.Lock()
-	c.inSendStatus--
-	c.mu.Unlock()
+	// Launch a new goroutine to avoid blocking the caller while the observer
+	// does its thing, which may result in a call back into the client.
+	go c.observer.SetControlClientStatus(new)
 }
 
 func (c *Auto) Login(t *tailcfg.Oauth2Token, flags LoginFlags) {
@@ -732,7 +729,6 @@ func (c *Auto) Shutdown() {
 	c.logf("client.Shutdown()")
 
 	c.mu.Lock()
-	inSendStatus := c.inSendStatus
 	closed := c.closed
 	direct := c.direct
 	if !closed {
@@ -742,7 +738,7 @@ func (c *Auto) Shutdown() {
 	}
 	c.mu.Unlock()
 
-	c.logf("client.Shutdown: inSendStatus=%v", inSendStatus)
+	c.logf("client.Shutdown")
 	if !closed {
 		c.unregisterHealthWatch()
 		close(c.quit)
diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go
index d9faf8e00..340303031 100644
--- a/ipn/ipnlocal/local.go
+++ b/ipn/ipnlocal/local.go
@@ -1396,6 +1396,7 @@ func (b *LocalBackend) Start(opts ipn.Options) error {
 	wantRunning := prefs.WantRunning()
 	if wantRunning {
 		if err := b.initMachineKeyLocked(); err != nil {
+			b.mu.Unlock()
 			return fmt.Errorf("initMachineKeyLocked: %w", err)
 		}
 	}