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 0f604923d3, 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>
This commit is contained in:
Brad Fitzpatrick 2023-09-17 07:20:50 -05:00 committed by Brad Fitzpatrick
parent 8ab46952d4
commit 760b945bc0
2 changed files with 3 additions and 29 deletions

View File

@ -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)
}
}

View File

@ -15,7 +15,6 @@
"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