From 2fb9472990ea76b30b7ac7c138b856ba9500dfa1 Mon Sep 17 00:00:00 2001 From: "M. J. Fromberger" Date: Mon, 25 Aug 2025 10:49:06 -0700 Subject: [PATCH] ipn/ipnlocal: remove unnecessary usees of lockAndGetUnlock In places where we are locking the LocakBackend and immediately deferring an unlock, and where there is no shortcut path in the control flow below the deferral, we do not need the unlockOnce helper. Replace all these with use of the lock directly. Updates #11649 Change-Id: I3e6a7110dfc9ec6c1d38d2585c5367a0d4e76514 Signed-off-by: M. J. Fromberger --- ipn/ipnlocal/local.go | 72 +++++++++++++++++++++---------------------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 5e6724701..a5c4e1f22 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -797,8 +797,8 @@ func (b *LocalBackend) Dialer() *tsdial.Dialer { // It returns (false, nil) if not running in declarative mode, (true, nil) on // success, or (false, error) on failure. func (b *LocalBackend) ReloadConfig() (ok bool, err error) { - unlock := b.lockAndGetUnlock() - defer unlock() + b.mu.Lock() + defer b.mu.Unlock() if b.conf == nil { return false, nil } @@ -1956,8 +1956,8 @@ func (b *LocalBackend) registerSysPolicyWatch() (unregister func(), err error) { // // b.mu must not be held. func (b *LocalBackend) reconcilePrefs() (_ ipn.PrefsView, anyChange bool) { - unlock := b.lockAndGetUnlock() - defer unlock() + b.mu.Lock() + defer b.mu.Unlock() prefs := b.pm.CurrentPrefs().AsStruct() if !b.reconcilePrefsLocked(prefs) { return prefs.View(), false @@ -2284,8 +2284,8 @@ func (b *LocalBackend) Start(opts ipn.Options) error { clientToShutdown.Shutdown() } }() - unlock := b.lockAndGetUnlock() - defer unlock() + b.mu.Lock() + defer b.mu.Unlock() if opts.UpdatePrefs != nil { if err := b.checkPrefsLocked(opts.UpdatePrefs); err != nil { @@ -3486,8 +3486,8 @@ func (b *LocalBackend) onClientVersion(v *tailcfg.ClientVersion) { } func (b *LocalBackend) onTailnetDefaultAutoUpdate(au bool) { - unlock := b.lockAndGetUnlock() - defer unlock() + b.mu.Lock() + defer b.mu.Unlock() prefs := b.pm.CurrentPrefs() if !prefs.Valid() { @@ -3953,8 +3953,8 @@ func (b *LocalBackend) shouldUploadServices() bool { // // On non-multi-user systems, the actor should be set to nil. func (b *LocalBackend) SetCurrentUser(actor ipnauth.Actor) { - unlock := b.lockAndGetUnlock() - defer unlock() + b.mu.Lock() + defer b.mu.Unlock() var userIdentifier string if user := cmp.Or(actor, b.currentUser); user != nil { @@ -3986,8 +3986,8 @@ func (b *LocalBackend) SetCurrentUser(actor ipnauth.Actor) { // or disconnecting, or a change in the desktop session state, and is used // for logging. func (b *LocalBackend) SwitchToBestProfile(reason string) { - unlock := b.lockAndGetUnlock() - defer unlock() + b.mu.Lock() + defer b.mu.Unlock() b.switchToBestProfileLocked(reason) } @@ -4260,8 +4260,8 @@ func (b *LocalBackend) checkAutoUpdatePrefsLocked(p *ipn.Prefs) error { // Setting the value to false when use of an exit node is already false is not an error, // nor is true when the exit node is already in use. func (b *LocalBackend) SetUseExitNodeEnabled(actor ipnauth.Actor, v bool) (ipn.PrefsView, error) { - unlock := b.lockAndGetUnlock() - defer unlock() + b.mu.Lock() + defer b.mu.Unlock() p0 := b.pm.CurrentPrefs() if v && p0.ExitNodeID() != "" { @@ -4331,8 +4331,8 @@ func (b *LocalBackend) EditPrefsAs(mp *ipn.MaskedPrefs, actor ipnauth.Actor) (ip return ipn.PrefsView{}, errors.New("can't set Internal fields") } - unlock := b.lockAndGetUnlock() - defer unlock() + b.mu.Lock() + defer b.mu.Unlock() return b.editPrefsLocked(actor, mp) } @@ -4521,8 +4521,8 @@ func (b *LocalBackend) startReconnectTimerLocked(d time.Duration) { profileID := b.pm.CurrentProfile().ID() var reconnectTimer tstime.TimerController reconnectTimer = b.clock.AfterFunc(d, func() { - unlock := b.lockAndGetUnlock() - defer unlock() + b.mu.Lock() + defer b.mu.Unlock() if b.reconnectTimer != reconnectTimer { // We're either not the most recent timer, or we lost the race when @@ -4569,7 +4569,7 @@ func (b *LocalBackend) stopReconnectTimerLocked() { } } -// Warning: b.mu must be held on entry. +// The caller must hold b.mu. func (b *LocalBackend) editPrefsLocked(actor ipnauth.Actor, mp *ipn.MaskedPrefs) (ipn.PrefsView, error) { p0 := b.pm.CurrentPrefs() @@ -5616,8 +5616,8 @@ func (b *LocalBackend) applyPrefsToHostinfoLocked(hi *tailcfg.Hostinfo, prefs ip // really this is more "one of several places in which random things // happen". func (b *LocalBackend) enterState(newState ipn.State) { - unlock := b.lockAndGetUnlock() - defer unlock() + b.mu.Lock() + defer b.mu.Unlock() b.enterStateLocked(newState) } @@ -5820,8 +5820,8 @@ func (b *LocalBackend) nextStateLocked() ipn.State { // TODO(apenwarr): use a channel or something to prevent reentrancy? // Or maybe just call the state machine from fewer places. func (b *LocalBackend) stateMachine() { - unlock := b.lockAndGetUnlock() - defer unlock() + b.mu.Lock() + defer b.mu.Unlock() b.stateMachineLocked() } @@ -6015,8 +6015,8 @@ func (b *LocalBackend) Logout(ctx context.Context, actor ipnauth.Actor) error { var profile ipn.LoginProfileView if err := func() error { - unlock := b.lockAndGetUnlock() - defer unlock() + b.mu.Lock() + defer b.mu.Unlock() if !b.hasNodeKeyLocked() { // Already logged out. @@ -6058,8 +6058,8 @@ func (b *LocalBackend) Logout(ctx context.Context, actor ipnauth.Actor) error { return err } - unlock := b.lockAndGetUnlock() - defer unlock() + b.mu.Lock() + defer b.mu.Unlock() if err := b.pm.DeleteProfile(profile.ID()); err != nil { b.logf("error deleting profile: %v", err) @@ -7241,8 +7241,8 @@ func (b *LocalBackend) ShouldInterceptVIPServiceTCPPort(ap netip.AddrPort) bool // It will restart the backend on success. // If the profile is not known, it returns an errProfileNotFound. func (b *LocalBackend) SwitchProfile(profile ipn.ProfileID) error { - unlock := b.lockAndGetUnlock() - defer unlock() + b.mu.Lock() + defer b.mu.Unlock() oldControlURL := b.pm.CurrentPrefs().ControlURLOrDefault() if _, changed, err := b.pm.SwitchToProfileByID(profile); !changed || err != nil { @@ -7336,7 +7336,7 @@ func (b *LocalBackend) getHardwareAddrs() ([]string, error) { // resetForProfileChangeLocked resets the backend for a profile change. // -// b.mu must held on entry. It is released on exit. +// The caller must hold b.mu. func (b *LocalBackend) resetForProfileChangeLocked() error { if b.shutdownCalled { // Prevent a call back to Start during Shutdown, which calls Logout for @@ -7386,8 +7386,8 @@ func (b *LocalBackend) resetForProfileChangeLocked() error { // DeleteProfile deletes a profile with the given ID. // If the profile is not known, it is a no-op. func (b *LocalBackend) DeleteProfile(p ipn.ProfileID) error { - unlock := b.lockAndGetUnlock() - defer unlock() + b.mu.Lock() + defer b.mu.Unlock() needToRestart := b.pm.CurrentProfile().ID() == p if err := b.pm.DeleteProfile(p); err != nil { @@ -7412,8 +7412,8 @@ func (b *LocalBackend) CurrentProfile() ipn.LoginProfileView { // NewProfile creates and switches to the new profile. func (b *LocalBackend) NewProfile() error { - unlock := b.lockAndGetUnlock() - defer unlock() + b.mu.Lock() + defer b.mu.Unlock() b.pm.SwitchToNewProfile() @@ -7436,8 +7436,8 @@ func (b *LocalBackend) ListProfiles() []ipn.LoginProfileView { // backend is left with a new profile, ready for StartLoginInterative to be // called to register it as new node. func (b *LocalBackend) ResetAuth() error { - unlock := b.lockAndGetUnlock() - defer unlock() + b.mu.Lock() + defer b.mu.Unlock() prevCC := b.resetControlClientLocked() if prevCC != nil {