From 45d3174c0deafa061cb407c64d25d2c69679cb58 Mon Sep 17 00:00:00 2001
From: Josh Bleecher Snyder <josh@tailscale.com>
Date: Wed, 15 Sep 2021 12:59:47 -0700
Subject: [PATCH] ipn/ipnlocal: add LocalBackend.broadcastStatusChanged

To reduce repetition.

Also, document why we acquire the lock.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
---
 ipn/ipnlocal/local.go | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go
index 8bee8f815..a76296879 100644
--- a/ipn/ipnlocal/local.go
+++ b/ipn/ipnlocal/local.go
@@ -601,18 +601,12 @@ func (b *LocalBackend) findExitNodeIDLocked(nm *netmap.NetworkMap) (prefsChanged
 func (b *LocalBackend) setWgengineStatus(s *wgengine.Status, err error) {
 	if err != nil {
 		b.logf("wgengine status error: %v", err)
-
-		b.statusLock.Lock()
-		b.statusChanged.Broadcast()
-		b.statusLock.Unlock()
+		b.broadcastStatusChanged()
 		return
 	}
 	if s == nil {
 		b.logf("[unexpected] non-error wgengine update with status=nil: %v", s)
-
-		b.statusLock.Lock()
-		b.statusChanged.Broadcast()
-		b.statusLock.Unlock()
+		b.broadcastStatusChanged()
 		return
 	}
 
@@ -632,12 +626,18 @@ func (b *LocalBackend) setWgengineStatus(s *wgengine.Status, err error) {
 		}
 		b.stateMachine()
 	}
+	b.broadcastStatusChanged()
+	b.send(ipn.Notify{Engine: &es})
+}
 
+func (b *LocalBackend) broadcastStatusChanged() {
+	// The sync.Cond docs say: "It is allowed but not required for the caller to hold c.L during the call."
+	// In this particular case, we must acquire b.statusLock. Otherwise we might broadcast before
+	// the waiter (in requestEngineStatusAndWait) starts to wait, in which case
+	// the waiter can get stuck indefinitely. See PR 2865.
 	b.statusLock.Lock()
 	b.statusChanged.Broadcast()
 	b.statusLock.Unlock()
-
-	b.send(ipn.Notify{Engine: &es})
 }
 
 func endpointsEqual(x, y []tailcfg.Endpoint) bool {