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 <fromberger@tailscale.com>

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()
+	b.mu.Lock()
-	defer unlock()
+	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()
+	b.mu.Lock()
-	defer unlock()
+	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()
+	b.mu.Lock()
-	defer unlock()
+	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()
+	b.mu.Lock()
-	defer unlock()
+	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()
+	b.mu.Lock()
-	defer unlock()
+	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()
+	b.mu.Lock()
-	defer unlock()
+	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()
+	b.mu.Lock()
-	defer unlock()
+	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()
+	b.mu.Lock()
-	defer unlock()
+	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()
+		b.mu.Lock()
-		defer unlock()
+		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()
+	b.mu.Lock()
-	defer unlock()
+	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()
+	b.mu.Lock()
-	defer unlock()
+	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()
+		b.mu.Lock()
-		defer unlock()
+		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()
+	b.mu.Lock()
-	defer unlock()
+	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()
+	b.mu.Lock()
-	defer unlock()
+	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()
+	b.mu.Lock()
-	defer unlock()
+	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()
+	b.mu.Lock()
-	defer unlock()
+	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()
+	b.mu.Lock()
-	defer unlock()
+	defer b.mu.Unlock()
 
 	prevCC := b.resetControlClientLocked()
 	if prevCC != nil {