From 760b945bc0ef87a49d3361da3e9e7c4e1036d490 Mon Sep 17 00:00:00 2001
From: Brad Fitzpatrick <bradfitz@tailscale.com>
Date: Sun, 17 Sep 2023 07:20:50 -0500
Subject: [PATCH] ipn/{ipnlocal,ipnstate}: start simplifying
 UpdateStatus/StatusBuilder

* Remove unnecessary mutexes (there's no concurrency)
* Simplify LocalBackend.UpdateStatus using the StatusBuilder.WantPeers
  field that was added in 0f604923d345ff, removing passing around some
  method values into func args. And then merge two methods.

More remains, but this is a start.

Updates #9433

Change-Id: Iaf2d7ec6e4e590799f00bae185465a4fd089b822
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
---
 ipn/ipnlocal/local.go    | 16 +++-------------
 ipn/ipnstate/ipnstate.go | 16 ----------------
 2 files changed, 3 insertions(+), 29 deletions(-)

diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go
index 40ab81447..70e56696c 100644
--- a/ipn/ipnlocal/local.go
+++ b/ipn/ipnlocal/local.go
@@ -650,18 +650,8 @@ func (b *LocalBackend) StatusWithoutPeers() *ipnstate.Status {
 
 // UpdateStatus implements ipnstate.StatusUpdater.
 func (b *LocalBackend) UpdateStatus(sb *ipnstate.StatusBuilder) {
-	b.e.UpdateStatus(sb)
-	var extraLocked func(*ipnstate.StatusBuilder)
-	if sb.WantPeers {
-		extraLocked = b.populatePeerStatusLocked
-	}
-	b.updateStatus(sb, extraLocked)
-}
+	b.e.UpdateStatus(sb) // does wireguard + magicsock status
 
-// updateStatus populates sb with status.
-//
-// extraLocked, if non-nil, is called while b.mu is still held.
-func (b *LocalBackend) updateStatus(sb *ipnstate.StatusBuilder, extraLocked func(*ipnstate.StatusBuilder)) {
 	b.mu.Lock()
 	defer b.mu.Unlock()
 
@@ -759,8 +749,8 @@ func (b *LocalBackend) updateStatus(sb *ipnstate.StatusBuilder, extraLocked func
 	// TODO: hostinfo, and its networkinfo
 	// TODO: EngineStatus copy (and deprecate it?)
 
-	if extraLocked != nil {
-		extraLocked(sb)
+	if sb.WantPeers {
+		b.populatePeerStatusLocked(sb)
 	}
 }
 
diff --git a/ipn/ipnstate/ipnstate.go b/ipn/ipnstate/ipnstate.go
index b7fc2ca06..125378610 100644
--- a/ipn/ipnstate/ipnstate.go
+++ b/ipn/ipnstate/ipnstate.go
@@ -15,7 +15,6 @@ import (
 	"slices"
 	"sort"
 	"strings"
-	"sync"
 	"time"
 
 	"tailscale.com/tailcfg"
@@ -298,7 +297,6 @@ type PeerStatus struct {
 type StatusBuilder struct {
 	WantPeers bool // whether caller wants peers
 
-	mu     sync.Mutex
 	locked bool
 	st     Status
 }
@@ -307,19 +305,13 @@ type StatusBuilder struct {
 //
 // It may not assume other fields of status are already populated, and
 // may not retain or write to the Status after f returns.
-//
-// MutateStatus acquires a lock so f must not call back into sb.
 func (sb *StatusBuilder) MutateStatus(f func(*Status)) {
-	sb.mu.Lock()
-	defer sb.mu.Unlock()
 	f(&sb.st)
 }
 
 // Status returns the status that has been built up so far from previous
 // calls to MutateStatus, MutateSelfStatus, AddPeer, etc.
 func (sb *StatusBuilder) Status() *Status {
-	sb.mu.Lock()
-	defer sb.mu.Unlock()
 	sb.locked = true
 	return &sb.st
 }
@@ -331,8 +323,6 @@ func (sb *StatusBuilder) Status() *Status {
 //
 // MutateStatus acquires a lock so f must not call back into sb.
 func (sb *StatusBuilder) MutateSelfStatus(f func(*PeerStatus)) {
-	sb.mu.Lock()
-	defer sb.mu.Unlock()
 	if sb.st.Self == nil {
 		sb.st.Self = new(PeerStatus)
 	}
@@ -341,8 +331,6 @@ func (sb *StatusBuilder) MutateSelfStatus(f func(*PeerStatus)) {
 
 // AddUser adds a user profile to the status.
 func (sb *StatusBuilder) AddUser(id tailcfg.UserID, up tailcfg.UserProfile) {
-	sb.mu.Lock()
-	defer sb.mu.Unlock()
 	if sb.locked {
 		log.Printf("[unexpected] ipnstate: AddUser after Locked")
 		return
@@ -357,8 +345,6 @@ func (sb *StatusBuilder) AddUser(id tailcfg.UserID, up tailcfg.UserProfile) {
 
 // AddIP adds a Tailscale IP address to the status.
 func (sb *StatusBuilder) AddTailscaleIP(ip netip.Addr) {
-	sb.mu.Lock()
-	defer sb.mu.Unlock()
 	if sb.locked {
 		log.Printf("[unexpected] ipnstate: AddIP after Locked")
 		return
@@ -375,8 +361,6 @@ func (sb *StatusBuilder) AddPeer(peer key.NodePublic, st *PeerStatus) {
 		panic("nil PeerStatus")
 	}
 
-	sb.mu.Lock()
-	defer sb.mu.Unlock()
 	if sb.locked {
 		log.Printf("[unexpected] ipnstate: AddPeer after Locked")
 		return