Revert "ipn/ipnlocal: replace the LockedOnEntry pattern with conventional lock/unlock discipline (#16925)"

This reverts commit 6c8fef961eab77a51e2b30dcce0f84d7478892b2. Reason: rolling back atypical `*Locked` methods. Updates tailscale/corp#31713 Updates #11649 Signed-off-by: Nick Khyl <nickk@tailscale.com>
2026-05-05 20:26:47 +02:00 · 2025-08-27 13:06:07 -05:00 · 2025-08-27 13:06:07 -05:00 · b95987e84c
commit b95987e84c
parent fa0af2afdf
3 changed files with 160 additions and 173 deletions
--- a/ipn/ipnlocal/local.go
+++ b/ipn/ipnlocal/local.go
@ -806,7 +806,7 @@ func (b *LocalBackend) ReloadConfig() (ok bool, err error) {
 	if err != nil {
 		return false, err
 	}
-	if err := b.setConfigLocked(conf); err != nil {
+	if err := b.setConfigLockedOnEntry(conf, unlock); err != nil {
 		return false, fmt.Errorf("error setting config: %w", err)
 	}

@ -863,9 +863,10 @@ func (b *LocalBackend) setStateLocked(state ipn.State) {
 	}
 }

-// setConfigLocked uses the provided config to update the backend's prefs
+// setConfigLockedOnEntry uses the provided config to update the backend's prefs
 // and other state.
-func (b *LocalBackend) setConfigLocked(conf *conffile.Config) error {
+func (b *LocalBackend) setConfigLockedOnEntry(conf *conffile.Config, unlock unlockOnce) error {
+	defer unlock()
 	p := b.pm.CurrentPrefs().AsStruct()
 	mp, err := conf.Parsed.ToPrefs()
 	if err != nil {
@ -873,7 +874,8 @@ func (b *LocalBackend) setConfigLocked(conf *conffile.Config) error {
 	}
 	p.ApplyEdits(&mp)
 	b.setStaticEndpointsFromConfigLocked(conf)
-	b.setPrefsLocked(p)
+	b.setPrefsLockedOnEntry(p, unlock)
+
 	b.conf = conf
 	return nil
 }
@ -1959,12 +1961,12 @@ 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()
 	prefs := b.pm.CurrentPrefs().AsStruct()
 	if !b.reconcilePrefsLocked(prefs) {
+		unlock.UnlockEarly()
 		return prefs.View(), false
 	}
-	return b.setPrefsLocked(prefs), true
+	return b.setPrefsLockedOnEntry(prefs, unlock), true
 }

 // sysPolicyChanged is a callback triggered by syspolicy when it detects
@ -2533,7 +2535,8 @@ func (b *LocalBackend) Start(opts ipn.Options) error {
 		// regress tsnet.Server restarts.
 		cc.Login(controlclient.LoginDefault)
 	}
-	b.stateMachineLocked()
+	b.stateMachineLockedOnEntry(unlock)
+
 	return nil
 }

@ -3557,14 +3560,14 @@ func (b *LocalBackend) onTailnetDefaultAutoUpdate(au bool) {
 		b.logf("using tailnet default auto-update setting: %v", au)
 		prefsClone := prefs.AsStruct()
 		prefsClone.AutoUpdate.Apply = opt.NewBool(au)
-		_, err := b.editPrefsLocked(
+		_, err := b.editPrefsLockedOnEntry(
 			ipnauth.Self,
 			&ipn.MaskedPrefs{
 				Prefs: *prefsClone,
 				AutoUpdateSet: ipn.AutoUpdatePrefsMask{
 					ApplySet: true,
 				},
-			})
+			}, unlock)
 		if err != nil {
 			b.logf("failed to apply tailnet-wide default for auto-updates (%v): %v", au, err)
 			return
@ -4024,7 +4027,7 @@ func (b *LocalBackend) SetCurrentUser(actor ipnauth.Actor) {
 		action = "connected"
 	}
 	reason := fmt.Sprintf("client %s (%s)", action, userIdentifier)
-	b.switchToBestProfileLocked(reason)
+	b.switchToBestProfileLockedOnEntry(reason, unlock)
 }

 // SwitchToBestProfile selects the best profile to use,
@ -4034,14 +4037,13 @@ 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.switchToBestProfileLocked(reason)
+	b.switchToBestProfileLockedOnEntry(reason, b.lockAndGetUnlock())
 }

-// switchToBestProfileLocked is like [LocalBackend.SwitchToBestProfile], but
-// the caller must hold b.mu.
-func (b *LocalBackend) switchToBestProfileLocked(reason string) {
+// switchToBestProfileLockedOnEntry is like [LocalBackend.SwitchToBestProfile],
+// but b.mu must held on entry. It is released on exit.
+func (b *LocalBackend) switchToBestProfileLockedOnEntry(reason string, unlock unlockOnce) {
+	defer unlock()
 	oldControlURL := b.pm.CurrentPrefs().ControlURLOrDefault()
 	profile, background := b.resolveBestProfileLocked()
 	cp, switched, err := b.pm.SwitchToProfile(profile)
@ -4072,7 +4074,7 @@ func (b *LocalBackend) switchToBestProfileLocked(reason string) {
 	if newControlURL := b.pm.CurrentPrefs().ControlURLOrDefault(); oldControlURL != newControlURL {
 		b.resetDialPlan()
 	}
-	if err := b.resetForProfileChangeLocked(); err != nil {
+	if err := b.resetForProfileChangeLockedOnEntry(unlock); err != nil {
 		// TODO(nickkhyl): The actual reset cannot fail. However,
 		// the TKA initialization or [LocalBackend.Start] can fail.
 		// These errors are not critical as far as we're concerned.
@ -4350,7 +4352,7 @@ func (b *LocalBackend) SetUseExitNodeEnabled(actor ipnauth.Actor, v bool) (ipn.P
 			mp.InternalExitNodePrior = p0.ExitNodeID()
 		}
 	}
-	return b.editPrefsLocked(actor, mp)
+	return b.editPrefsLockedOnEntry(actor, mp, unlock)
 }

 // MaybeClearAppConnector clears the routes from any AppConnector if
@ -4379,9 +4381,7 @@ 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()
-	return b.editPrefsLocked(actor, mp)
+	return b.editPrefsLockedOnEntry(actor, mp, b.lockAndGetUnlock())
 }

 // checkEditPrefsAccessLocked checks whether the current user has access
@ -4588,7 +4588,7 @@ func (b *LocalBackend) startReconnectTimerLocked(d time.Duration) {
 		}

 		mp := &ipn.MaskedPrefs{WantRunningSet: true, Prefs: ipn.Prefs{WantRunning: true}}
-		if _, err := b.editPrefsLocked(ipnauth.Self, mp); err != nil {
+		if _, err := b.editPrefsLockedOnEntry(ipnauth.Self, mp, unlock); err != nil {
 			b.logf("failed to automatically reconnect as %q after %v: %v", cp.Name(), d, err)
 		} else {
 			b.logf("automatically reconnected as %q after %v", cp.Name(), d)
@ -4617,8 +4617,11 @@ func (b *LocalBackend) stopReconnectTimerLocked() {
 	}
 }

-// Warning: b.mu must be held on entry.
-func (b *LocalBackend) editPrefsLocked(actor ipnauth.Actor, mp *ipn.MaskedPrefs) (ipn.PrefsView, error) {
+// Warning: b.mu must be held on entry, but it unlocks it on the way out.
+// TODO(bradfitz): redo the locking on all these weird methods like this.
+func (b *LocalBackend) editPrefsLockedOnEntry(actor ipnauth.Actor, mp *ipn.MaskedPrefs, unlock unlockOnce) (ipn.PrefsView, error) {
+	defer unlock() // for error paths
+
 	p0 := b.pm.CurrentPrefs()

 	// Check if the changes in mp are allowed.
@ -4655,10 +4658,12 @@ func (b *LocalBackend) editPrefsLocked(actor ipnauth.Actor, mp *ipn.MaskedPrefs)
 	// before the modified prefs are actually set for the current profile.
 	b.onEditPrefsLocked(actor, mp, p0, p1.View())

-	newPrefs := b.setPrefsLocked(p1)
+	newPrefs := b.setPrefsLockedOnEntry(p1, unlock)
+
+	// Note: don't perform any actions for the new prefs here. Not
+	// every prefs change goes through EditPrefs. Put your actions
+	// in setPrefsLocksOnEntry instead.

-	// Note: don't perform any actions for the new prefs here. Not every prefs
-	// change goes through EditPrefs. Put your actions in setPrefsLocked instead.
 	// This should return the public prefs, not the private ones.
 	return stripKeysFromPrefs(newPrefs), nil
 }
@ -4706,9 +4711,12 @@ func (b *LocalBackend) shouldWireInactiveIngressLocked() bool {
 	return b.serveConfig.Valid() && !b.hasIngressEnabledLocked() && b.wantIngressLocked()
 }

-// setPrefsLocked requires b.mu be held to call it.  It returns a read-only
-// copy of the new prefs.
-func (b *LocalBackend) setPrefsLocked(newp *ipn.Prefs) ipn.PrefsView {
+// setPrefsLockedOnEntry requires b.mu be held to call it, but it
+// unlocks b.mu when done. newp ownership passes to this function.
+// It returns a read-only copy of the new prefs.
+func (b *LocalBackend) setPrefsLockedOnEntry(newp *ipn.Prefs, unlock unlockOnce) ipn.PrefsView {
+	defer unlock()
+
 	cn := b.currentNode()
 	netMap := cn.NetMap()
 	b.setAtomicValuesFromPrefsLocked(newp.View())
@ -4777,33 +4785,28 @@ func (b *LocalBackend) setPrefsLocked(newp *ipn.Prefs) ipn.PrefsView {
 		b.stopOfflineAutoUpdate()
 	}

-	// Update status that needs to happen outside the lock, but reacquire it
-	// before returning (including in case of panics).
-	func() {
-		b.mu.Unlock()
-		defer b.mu.Lock()
+	unlock.UnlockEarly()

-		if oldp.ShieldsUp() != newp.ShieldsUp || hostInfoChanged {
-			b.doSetHostinfoFilterServices()
-		}
+	if oldp.ShieldsUp() != newp.ShieldsUp || hostInfoChanged {
+		b.doSetHostinfoFilterServices()
+	}

-		if netMap != nil {
-			b.MagicConn().SetDERPMap(netMap.DERPMap)
-		}
+	if netMap != nil {
+		b.MagicConn().SetDERPMap(netMap.DERPMap)
+	}

-		if !oldp.WantRunning() && newp.WantRunning && cc != nil {
-			b.logf("transitioning to running; doing Login...")
-			cc.Login(controlclient.LoginDefault)
-		}
+	if !oldp.WantRunning() && newp.WantRunning && cc != nil {
+		b.logf("transitioning to running; doing Login...")
+		cc.Login(controlclient.LoginDefault)
+	}

-		if oldp.WantRunning() != newp.WantRunning {
-			b.stateMachine()
-		} else {
-			b.authReconfig()
-		}
+	if oldp.WantRunning() != newp.WantRunning {
+		b.stateMachine()
+	} else {
+		b.authReconfig()
+	}

-		b.send(ipn.Notify{Prefs: &prefs})
-	}()
+	b.send(ipn.Notify{Prefs: &prefs})
 	return prefs
 }

@ -5665,12 +5668,12 @@ func (b *LocalBackend) applyPrefsToHostinfoLocked(hi *tailcfg.Hostinfo, prefs ip
 // happen".
 func (b *LocalBackend) enterState(newState ipn.State) {
 	unlock := b.lockAndGetUnlock()
-	defer unlock()
-	b.enterStateLocked(newState)
+	b.enterStateLockedOnEntry(newState, unlock)
 }

-// enterStateLocked is like enterState but requires the caller to hold b.mu.
-func (b *LocalBackend) enterStateLocked(newState ipn.State) {
+// enterStateLockedOnEntry is like enterState but requires b.mu be held to call
+// it, but it unlocks b.mu when done (via unlock, a once func).
+func (b *LocalBackend) enterStateLockedOnEntry(newState ipn.State, unlock unlockOnce) {
 	cn := b.currentNode()
 	oldState := b.state
 	b.setStateLocked(newState)
@ -5719,56 +5722,51 @@ func (b *LocalBackend) enterStateLocked(newState ipn.State) {
 		b.maybeStartOfflineAutoUpdate(prefs)
 	}

-	// Resolve the state transition outside the lock, but reacquire it before
-	// returning (including in case of panics).
-	func() {
-		b.mu.Unlock()
-		defer b.mu.Lock()
+	unlock.UnlockEarly()

-		// prefs may change irrespective of state; WantRunning should be explicitly
-		// set before potential early return even if the state is unchanged.
-		b.health.SetIPNState(newState.String(), prefs.Valid() && prefs.WantRunning())
-		if oldState == newState {
-			return
+	// prefs may change irrespective of state; WantRunning should be explicitly
+	// set before potential early return even if the state is unchanged.
+	b.health.SetIPNState(newState.String(), prefs.Valid() && prefs.WantRunning())
+	if oldState == newState {
+		return
+	}
+	b.logf("Switching ipn state %v -> %v (WantRunning=%v, nm=%v)",
+		oldState, newState, prefs.WantRunning(), netMap != nil)
+	b.send(ipn.Notify{State: &newState})
+
+	switch newState {
+	case ipn.NeedsLogin:
+		systemd.Status("Needs login: %s", authURL)
+		if b.seamlessRenewalEnabled() {
+			break
 		}
-		b.logf("Switching ipn state %v -> %v (WantRunning=%v, nm=%v)",
-			oldState, newState, prefs.WantRunning(), netMap != nil)
-		b.send(ipn.Notify{State: &newState})
-
-		switch newState {
-		case ipn.NeedsLogin:
-			systemd.Status("Needs login: %s", authURL)
-			if b.seamlessRenewalEnabled() {
-				break
-			}
-			b.blockEngineUpdates(true)
-			fallthrough
-		case ipn.Stopped, ipn.NoState:
-			// Unconfigure the engine if it has stopped (WantRunning is set to false)
-			// or if we've switched to a different profile and the state is unknown.
-			err := b.e.Reconfig(&wgcfg.Config{}, &router.Config{}, &dns.Config{})
-			if err != nil {
-				b.logf("Reconfig(down): %v", err)
-			}
-
-			if newState == ipn.Stopped && authURL == "" {
-				systemd.Status("Stopped; run 'tailscale up' to log in")
-			}
-		case ipn.Starting, ipn.NeedsMachineAuth:
-			b.authReconfig()
-			// Needed so that UpdateEndpoints can run
-			b.e.RequestStatus()
-		case ipn.Running:
-			var addrStrs []string
-			addrs := netMap.GetAddresses()
-			for _, p := range addrs.All() {
-				addrStrs = append(addrStrs, p.Addr().String())
-			}
-			systemd.Status("Connected; %s; %s", activeLogin, strings.Join(addrStrs, " "))
-		default:
-			b.logf("[unexpected] unknown newState %#v", newState)
+		b.blockEngineUpdates(true)
+		fallthrough
+	case ipn.Stopped, ipn.NoState:
+		// Unconfigure the engine if it has stopped (WantRunning is set to false)
+		// or if we've switched to a different profile and the state is unknown.
+		err := b.e.Reconfig(&wgcfg.Config{}, &router.Config{}, &dns.Config{})
+		if err != nil {
+			b.logf("Reconfig(down): %v", err)
 		}
-	}()
+
+		if newState == ipn.Stopped && authURL == "" {
+			systemd.Status("Stopped; run 'tailscale up' to log in")
+		}
+	case ipn.Starting, ipn.NeedsMachineAuth:
+		b.authReconfig()
+		// Needed so that UpdateEndpoints can run
+		b.e.RequestStatus()
+	case ipn.Running:
+		var addrStrs []string
+		addrs := netMap.GetAddresses()
+		for _, p := range addrs.All() {
+			addrStrs = append(addrStrs, p.Addr().String())
+		}
+		systemd.Status("Connected; %s; %s", activeLogin, strings.Join(addrStrs, " "))
+	default:
+		b.logf("[unexpected] unknown newState %#v", newState)
+	}
 }

 func (b *LocalBackend) hasNodeKeyLocked() bool {
@ -5869,28 +5867,26 @@ func (b *LocalBackend) nextStateLocked() ipn.State {
 // Or maybe just call the state machine from fewer places.
 func (b *LocalBackend) stateMachine() {
 	unlock := b.lockAndGetUnlock()
-	defer unlock()
-	b.stateMachineLocked()
+	b.stateMachineLockedOnEntry(unlock)
 }

-// stateMachineLocked is like stateMachine but requires b.mu be held.
-func (b *LocalBackend) stateMachineLocked() {
-	b.enterStateLocked(b.nextStateLocked())
+// stateMachineLockedOnEntry is like stateMachine but requires b.mu be held to
+// call it, but it unlocks b.mu when done (via unlock, a once func).
+func (b *LocalBackend) stateMachineLockedOnEntry(unlock unlockOnce) {
+	b.enterStateLockedOnEntry(b.nextStateLocked(), unlock)
 }

-// lockAndGetUnlock locks b.mu and returns a function that will unlock it at
-// most once.
+// lockAndGetUnlock locks b.mu and returns a sync.OnceFunc function that will
+// unlock it at most once.
 //
-// TODO(creachadair): This was added as a guardrail against the unfortunate
-// "LockedOnEntry" methods that were originally used in this package (primarily
-// enterStateLockedOnEntry) that required b.mu held to be locked on entry to
-// the function but unlocked the mutex on their way out.
-//
-// Now that these have all been updated, we could remove this type and acquire
-// and release locks directly. For now, however, I've left it alone to reduce
-// the scope of lock-related changes.
-//
-// See: https://github.com/tailscale/tailscale/issues/11649
+// This is all very unfortunate but exists as a guardrail against the
+// unfortunate "lockedOnEntry" methods in this package (primarily
+// enterStateLockedOnEntry) that require b.mu held to be locked on entry to the
+// function but unlock the mutex on their way out. As a stepping stone to
+// cleaning things up (as of 2024-04-06), we at least pass the unlock func
+// around now and defer unlock in the caller to avoid missing unlocks and double
+// unlocks. TODO(bradfitz,maisem): make the locking in this package more
+// traditional (simple). See https://github.com/tailscale/tailscale/issues/11649
 func (b *LocalBackend) lockAndGetUnlock() (unlock unlockOnce) {
 	b.mu.Lock()
 	var unlocked atomic.Bool
@ -6058,35 +6054,30 @@ func (b *LocalBackend) ShouldHandleViaIP(ip netip.Addr) bool {
 // Logout logs out the current profile, if any, and waits for the logout to
 // complete.
 func (b *LocalBackend) Logout(ctx context.Context, actor ipnauth.Actor) error {
-	// These values are initialized inside the lock on success.
-	var cc controlclient.Client
-	var profile ipn.LoginProfileView
+	unlock := b.lockAndGetUnlock()
+	defer unlock()

-	if err := func() error {
-		unlock := b.lockAndGetUnlock()
-		defer unlock()
+	if !b.hasNodeKeyLocked() {
+		// Already logged out.
+		return nil
+	}
+	cc := b.cc

-		if !b.hasNodeKeyLocked() {
-			// Already logged out.
-			return nil
-		}
-		cc = b.cc
+	// Grab the current profile before we unlock the mutex, so that we can
+	// delete it later.
+	profile := b.pm.CurrentProfile()

-		// Grab the current profile before we unlock the mutex, so that we can
-		// delete it later.
-		profile = b.pm.CurrentProfile()
-
-		_, err := b.editPrefsLocked(
-			actor,
-			&ipn.MaskedPrefs{
-				WantRunningSet: true,
-				LoggedOutSet:   true,
-				Prefs:          ipn.Prefs{WantRunning: false, LoggedOut: true},
-			})
-		return err
-	}(); err != nil {
+	_, err := b.editPrefsLockedOnEntry(
+		actor,
+		&ipn.MaskedPrefs{
+			WantRunningSet: true,
+			LoggedOutSet:   true,
+			Prefs:          ipn.Prefs{WantRunning: false, LoggedOut: true},
+		}, unlock)
+	if err != nil {
 		return err
 	}
+	// b.mu is now unlocked, after editPrefsLockedOnEntry.

 	// Clear any previous dial plan(s), if set.
 	b.resetDialPlan()
@ -6106,14 +6097,14 @@ func (b *LocalBackend) Logout(ctx context.Context, actor ipnauth.Actor) error {
 		return err
 	}

-	unlock := b.lockAndGetUnlock()
+	unlock = b.lockAndGetUnlock()
 	defer unlock()

 	if err := b.pm.DeleteProfile(profile.ID()); err != nil {
 		b.logf("error deleting profile: %v", err)
 		return err
 	}
-	return b.resetForProfileChangeLocked()
+	return b.resetForProfileChangeLockedOnEntry(unlock)
 }

 // setNetInfo sets b.hostinfo.NetInfo to ni, and passes ni along to the
@ -7302,7 +7293,7 @@ func (b *LocalBackend) SwitchProfile(profile ipn.ProfileID) error {
 		b.resetDialPlan()
 	}

-	return b.resetForProfileChangeLocked()
+	return b.resetForProfileChangeLockedOnEntry(unlock)
 }

 func (b *LocalBackend) initTKALocked() error {
@ -7382,10 +7373,12 @@ func (b *LocalBackend) getHardwareAddrs() ([]string, error) {
 	return addrs, nil
 }

-// resetForProfileChangeLocked resets the backend for a profile change.
+// resetForProfileChangeLockedOnEntry resets the backend for a profile change.
 //
 // b.mu must held on entry. It is released on exit.
-func (b *LocalBackend) resetForProfileChangeLocked() error {
+func (b *LocalBackend) resetForProfileChangeLockedOnEntry(unlock unlockOnce) error {
+	defer unlock()
+
 	if b.shutdownCalled {
 		// Prevent a call back to Start during Shutdown, which calls Logout for
 		// ephemeral nodes, which can then call back here. But we're shutting
@ -7416,19 +7409,12 @@ func (b *LocalBackend) resetForProfileChangeLocked() error {
 	b.resetAlwaysOnOverrideLocked()
 	b.extHost.NotifyProfileChange(b.pm.CurrentProfile(), b.pm.CurrentPrefs(), false)
 	b.setAtomicValuesFromPrefsLocked(b.pm.CurrentPrefs())
-	b.enterStateLocked(ipn.NoState)
-
-	// Update health status and start outside the lock.
-	return func() error {
-		b.mu.Unlock()
-		defer b.mu.Lock()
-
-		b.health.SetLocalLogConfigHealth(nil)
-		if tkaErr != nil {
-			return tkaErr
-		}
-		return b.Start(ipn.Options{})
-	}()
+	b.enterStateLockedOnEntry(ipn.NoState, unlock) // Reset state; releases b.mu
+	b.health.SetLocalLogConfigHealth(nil)
+	if tkaErr != nil {
+		return tkaErr
+	}
+	return b.Start(ipn.Options{})
 }

 // DeleteProfile deletes a profile with the given ID.
@ -7447,7 +7433,7 @@ func (b *LocalBackend) DeleteProfile(p ipn.ProfileID) error {
 	if !needToRestart {
 		return nil
 	}
-	return b.resetForProfileChangeLocked()
+	return b.resetForProfileChangeLockedOnEntry(unlock)
 }

 // CurrentProfile returns the current LoginProfile.
@ -7469,7 +7455,7 @@ func (b *LocalBackend) NewProfile() error {
 	// set. Conservatively reset the dialPlan.
 	b.resetDialPlan()

-	return b.resetForProfileChangeLocked()
+	return b.resetForProfileChangeLockedOnEntry(unlock)
 }

 // ListProfiles returns a list of all LoginProfiles.
@ -7498,7 +7484,7 @@ func (b *LocalBackend) ResetAuth() error {
 		return err
 	}
 	b.resetDialPlan() // always reset if we're removing everything
-	return b.resetForProfileChangeLocked()
+	return b.resetForProfileChangeLockedOnEntry(unlock)
 }

 func (b *LocalBackend) GetPeerEndpointChanges(ctx context.Context, ip netip.Addr) ([]magicsock.EndpointChange, error) {
--- a/ipn/ipnlocal/local_test.go
+++ b/ipn/ipnlocal/local_test.go
@ -4300,7 +4300,7 @@ func (b *LocalBackend) SetPrefsForTest(newp *ipn.Prefs) {
 	}
 	unlock := b.lockAndGetUnlock()
 	defer unlock()
-	b.setPrefsLocked(newp)
+	b.setPrefsLockedOnEntry(newp, unlock)
 }

 type peerOptFunc func(*tailcfg.Node)
--- a/ipn/ipnlocal/profiles.go
+++ b/ipn/ipnlocal/profiles.go
@ -180,7 +180,7 @@ func (pm *profileManager) SwitchToProfile(profile ipn.LoginProfileView) (cp ipn.
 		f(pm.currentProfile, pm.prefs, false)
 	}
 	// Do not call pm.extHost.NotifyProfileChange here; it is invoked in
-	// [LocalBackend.resetForProfileChangeLocked] after the netmap reset.
+	// [LocalBackend.resetForProfileChangeLockedOnEntry] after the netmap reset.
 	// TODO(nickkhyl): Consider moving it here (or into the stateChangeCb handler
 	// in [LocalBackend]) once the profile/node state, including the netmap,
 	// is actually tied to the current profile.
@ -359,9 +359,9 @@ func (pm *profileManager) SetPrefs(prefsIn ipn.PrefsView, np ipn.NetworkProfile)
 	// where prefsIn is the previous profile's prefs with an updated Persist, LoggedOut,
 	// WantRunning and possibly other fields. This may not be the desired behavior.
 	//
-	// Additionally, LocalBackend doesn't treat it as a proper profile switch,
-	// meaning that [LocalBackend.resetForProfileChangeLocked] is not called and
-	// certain node/profile-specific state may not be reset as expected.
+	// Additionally, LocalBackend doesn't treat it as a proper profile switch, meaning that
+	// [LocalBackend.resetForProfileChangeLockedOnEntry] is not called and certain
+	// node/profile-specific state may not be reset as expected.
 	//
 	// However, [profileManager] notifies [ipnext.Extension]s about the profile change,
 	// so features migrated from LocalBackend to external packages should not be affected.
@ -494,9 +494,10 @@ func (pm *profileManager) setProfilePrefsNoPermCheck(profile ipn.LoginProfileVie
 		oldPrefs := pm.prefs
 		pm.prefs = clonedPrefs

-		// Sadly, profile prefs can be changed in multiple ways.  It's pretty
-		// chaotic, and in many cases callers use unexported methods of the
-		// profile manager instead of going through [LocalBackend.setPrefsLocked]
+		// Sadly, profile prefs can be changed in multiple ways.
+		// It's pretty chaotic, and in many cases callers use
+		// unexported methods of the profile manager instead of
+		// going through [LocalBackend.setPrefsLockedOnEntry]
 		// or at least using [profileManager.SetPrefs].
 		//
 		// While we should definitely clean this up to improve