From b95987e84cbf2fbf60c0d3c72b06938bd084e79c Mon Sep 17 00:00:00 2001 From: Nick Khyl Date: Wed, 27 Aug 2025 13:06:07 -0500 Subject: [PATCH] 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 --- ipn/ipnlocal/local.go | 316 ++++++++++++++++++------------------- ipn/ipnlocal/local_test.go | 2 +- ipn/ipnlocal/profiles.go | 15 +- 3 files changed, 160 insertions(+), 173 deletions(-) diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 5e8ef17f3..25a81b017 100644 --- 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) { diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index 60b5b2c5b..49cfc3e07 100644 --- 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) diff --git a/ipn/ipnlocal/profiles.go b/ipn/ipnlocal/profiles.go index 7519ee157..1d312cfa6 100644 --- 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