diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 5fb3d5771..5e6724701 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.setConfigLockedOnEntry(conf, unlock); err != nil { + if err := b.setConfigLocked(conf); err != nil { return false, fmt.Errorf("error setting config: %w", err) } @@ -863,10 +863,9 @@ func (b *LocalBackend) setStateLocked(state ipn.State) { } } -// setConfigLockedOnEntry uses the provided config to update the backend's prefs +// setConfigLocked uses the provided config to update the backend's prefs // and other state. -func (b *LocalBackend) setConfigLockedOnEntry(conf *conffile.Config, unlock unlockOnce) error { - defer unlock() +func (b *LocalBackend) setConfigLocked(conf *conffile.Config) error { p := b.pm.CurrentPrefs().AsStruct() mp, err := conf.Parsed.ToPrefs() if err != nil { @@ -874,8 +873,7 @@ func (b *LocalBackend) setConfigLockedOnEntry(conf *conffile.Config, unlock unlo } p.ApplyEdits(&mp) b.setStaticEndpointsFromConfigLocked(conf) - b.setPrefsLockedOnEntry(p, unlock) - + b.setPrefsLocked(p) b.conf = conf return nil } @@ -1959,12 +1957,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.setPrefsLockedOnEntry(prefs, unlock), true + return b.setPrefsLocked(prefs), true } // sysPolicyChanged is a callback triggered by syspolicy when it detects @@ -2492,8 +2490,7 @@ func (b *LocalBackend) Start(opts ipn.Options) error { // regress tsnet.Server restarts. cc.Login(controlclient.LoginDefault) } - b.stateMachineLockedOnEntry(unlock) - + b.stateMachineLocked() return nil } @@ -3512,14 +3509,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.editPrefsLockedOnEntry( + _, err := b.editPrefsLocked( 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 @@ -3979,7 +3976,7 @@ func (b *LocalBackend) SetCurrentUser(actor ipnauth.Actor) { action = "connected" } reason := fmt.Sprintf("client %s (%s)", action, userIdentifier) - b.switchToBestProfileLockedOnEntry(reason, unlock) + b.switchToBestProfileLocked(reason) } // SwitchToBestProfile selects the best profile to use, @@ -3989,13 +3986,14 @@ 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) { - b.switchToBestProfileLockedOnEntry(reason, b.lockAndGetUnlock()) + unlock := b.lockAndGetUnlock() + defer unlock() + b.switchToBestProfileLocked(reason) } -// 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() +// switchToBestProfileLocked is like [LocalBackend.SwitchToBestProfile], but +// the caller must hold b.mu. +func (b *LocalBackend) switchToBestProfileLocked(reason string) { oldControlURL := b.pm.CurrentPrefs().ControlURLOrDefault() profile, background := b.resolveBestProfileLocked() cp, switched, err := b.pm.SwitchToProfile(profile) @@ -4026,7 +4024,7 @@ func (b *LocalBackend) switchToBestProfileLockedOnEntry(reason string, unlock un if newControlURL := b.pm.CurrentPrefs().ControlURLOrDefault(); oldControlURL != newControlURL { b.resetDialPlan() } - if err := b.resetForProfileChangeLockedOnEntry(unlock); err != nil { + if err := b.resetForProfileChangeLocked(); 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. @@ -4304,7 +4302,7 @@ func (b *LocalBackend) SetUseExitNodeEnabled(actor ipnauth.Actor, v bool) (ipn.P mp.InternalExitNodePrior = p0.ExitNodeID() } } - return b.editPrefsLockedOnEntry(actor, mp, unlock) + return b.editPrefsLocked(actor, mp) } // MaybeClearAppConnector clears the routes from any AppConnector if @@ -4333,7 +4331,9 @@ func (b *LocalBackend) EditPrefsAs(mp *ipn.MaskedPrefs, actor ipnauth.Actor) (ip return ipn.PrefsView{}, errors.New("can't set Internal fields") } - return b.editPrefsLockedOnEntry(actor, mp, b.lockAndGetUnlock()) + unlock := b.lockAndGetUnlock() + defer unlock() + return b.editPrefsLocked(actor, mp) } // checkEditPrefsAccessLocked checks whether the current user has access @@ -4540,7 +4540,7 @@ func (b *LocalBackend) startReconnectTimerLocked(d time.Duration) { } mp := &ipn.MaskedPrefs{WantRunningSet: true, Prefs: ipn.Prefs{WantRunning: true}} - if _, err := b.editPrefsLockedOnEntry(ipnauth.Self, mp, unlock); err != nil { + if _, err := b.editPrefsLocked(ipnauth.Self, mp); 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) @@ -4569,11 +4569,8 @@ func (b *LocalBackend) stopReconnectTimerLocked() { } } -// 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 - +// Warning: b.mu must be held on entry. +func (b *LocalBackend) editPrefsLocked(actor ipnauth.Actor, mp *ipn.MaskedPrefs) (ipn.PrefsView, error) { p0 := b.pm.CurrentPrefs() // Check if the changes in mp are allowed. @@ -4610,12 +4607,10 @@ func (b *LocalBackend) editPrefsLockedOnEntry(actor ipnauth.Actor, mp *ipn.Maske // before the modified prefs are actually set for the current profile. b.onEditPrefsLocked(actor, mp, p0, p1.View()) - 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. + newPrefs := b.setPrefsLocked(p1) + // 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 } @@ -4663,12 +4658,9 @@ func (b *LocalBackend) shouldWireInactiveIngressLocked() bool { return b.serveConfig.Valid() && !b.hasIngressEnabledLocked() && b.wantIngressLocked() } -// 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() - +// 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 { cn := b.currentNode() netMap := cn.NetMap() b.setAtomicValuesFromPrefsLocked(newp.View()) @@ -4737,28 +4729,33 @@ func (b *LocalBackend) setPrefsLockedOnEntry(newp *ipn.Prefs, unlock unlockOnce) b.stopOfflineAutoUpdate() } - unlock.UnlockEarly() + // 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() - 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 } @@ -5620,12 +5617,12 @@ func (b *LocalBackend) applyPrefsToHostinfoLocked(hi *tailcfg.Hostinfo, prefs ip // happen". func (b *LocalBackend) enterState(newState ipn.State) { unlock := b.lockAndGetUnlock() - b.enterStateLockedOnEntry(newState, unlock) + defer unlock() + b.enterStateLocked(newState) } -// 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) { +// enterStateLocked is like enterState but requires the caller to hold b.mu. +func (b *LocalBackend) enterStateLocked(newState ipn.State) { cn := b.currentNode() oldState := b.state b.setStateLocked(newState) @@ -5674,51 +5671,56 @@ func (b *LocalBackend) enterStateLockedOnEntry(newState ipn.State, unlock unlock b.maybeStartOfflineAutoUpdate(prefs) } - 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 - } - b.logf("Switching ipn state %v -> %v (WantRunning=%v, nm=%v)", - oldState, newState, prefs.WantRunning(), netMap != nil) - b.send(ipn.Notify{State: &newState}) + // 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() - 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) + // 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.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") + 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) } - 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 { @@ -5819,26 +5821,28 @@ func (b *LocalBackend) nextStateLocked() ipn.State { // Or maybe just call the state machine from fewer places. func (b *LocalBackend) stateMachine() { unlock := b.lockAndGetUnlock() - b.stateMachineLockedOnEntry(unlock) + defer unlock() + b.stateMachineLocked() } -// 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) +// stateMachineLocked is like stateMachine but requires b.mu be held. +func (b *LocalBackend) stateMachineLocked() { + b.enterStateLocked(b.nextStateLocked()) } -// lockAndGetUnlock locks b.mu and returns a sync.OnceFunc function that will -// unlock it at most once. +// lockAndGetUnlock locks b.mu and returns a 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. // -// 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 +// 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 func (b *LocalBackend) lockAndGetUnlock() (unlock unlockOnce) { b.mu.Lock() var unlocked atomic.Bool @@ -6006,30 +6010,35 @@ 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 { - unlock := b.lockAndGetUnlock() - defer unlock() + // These values are initialized inside the lock on success. + var cc controlclient.Client + var profile ipn.LoginProfileView - if !b.hasNodeKeyLocked() { - // Already logged out. - return nil - } - cc := b.cc + if err := func() error { + unlock := b.lockAndGetUnlock() + defer unlock() - // Grab the current profile before we unlock the mutex, so that we can - // delete it later. - profile := b.pm.CurrentProfile() - - _, err := b.editPrefsLockedOnEntry( - actor, - &ipn.MaskedPrefs{ - WantRunningSet: true, - LoggedOutSet: true, - Prefs: ipn.Prefs{WantRunning: false, LoggedOut: true}, - }, unlock) - if err != nil { + 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() + + _, err := b.editPrefsLocked( + actor, + &ipn.MaskedPrefs{ + WantRunningSet: true, + LoggedOutSet: true, + Prefs: ipn.Prefs{WantRunning: false, LoggedOut: true}, + }) + return err + }(); err != nil { return err } - // b.mu is now unlocked, after editPrefsLockedOnEntry. // Clear any previous dial plan(s), if set. b.resetDialPlan() @@ -6049,14 +6058,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.resetForProfileChangeLockedOnEntry(unlock) + return b.resetForProfileChangeLocked() } // setNetInfo sets b.hostinfo.NetInfo to ni, and passes ni along to the @@ -7245,7 +7254,7 @@ func (b *LocalBackend) SwitchProfile(profile ipn.ProfileID) error { b.resetDialPlan() } - return b.resetForProfileChangeLockedOnEntry(unlock) + return b.resetForProfileChangeLocked() } func (b *LocalBackend) initTKALocked() error { @@ -7325,12 +7334,10 @@ func (b *LocalBackend) getHardwareAddrs() ([]string, error) { return addrs, nil } -// resetForProfileChangeLockedOnEntry resets the backend for a profile change. +// resetForProfileChangeLocked resets the backend for a profile change. // // b.mu must held on entry. It is released on exit. -func (b *LocalBackend) resetForProfileChangeLockedOnEntry(unlock unlockOnce) error { - defer unlock() - +func (b *LocalBackend) resetForProfileChangeLocked() error { 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 @@ -7361,12 +7368,19 @@ func (b *LocalBackend) resetForProfileChangeLockedOnEntry(unlock unlockOnce) err b.resetAlwaysOnOverrideLocked() b.extHost.NotifyProfileChange(b.pm.CurrentProfile(), b.pm.CurrentPrefs(), false) b.setAtomicValuesFromPrefsLocked(b.pm.CurrentPrefs()) - b.enterStateLockedOnEntry(ipn.NoState, unlock) // Reset state; releases b.mu - b.health.SetLocalLogConfigHealth(nil) - if tkaErr != nil { - return tkaErr - } - return b.Start(ipn.Options{}) + 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{}) + }() } // DeleteProfile deletes a profile with the given ID. @@ -7385,7 +7399,7 @@ func (b *LocalBackend) DeleteProfile(p ipn.ProfileID) error { if !needToRestart { return nil } - return b.resetForProfileChangeLockedOnEntry(unlock) + return b.resetForProfileChangeLocked() } // CurrentProfile returns the current LoginProfile. @@ -7407,7 +7421,7 @@ func (b *LocalBackend) NewProfile() error { // set. Conservatively reset the dialPlan. b.resetDialPlan() - return b.resetForProfileChangeLockedOnEntry(unlock) + return b.resetForProfileChangeLocked() } // ListProfiles returns a list of all LoginProfiles. @@ -7436,7 +7450,7 @@ func (b *LocalBackend) ResetAuth() error { return err } b.resetDialPlan() // always reset if we're removing everything - return b.resetForProfileChangeLockedOnEntry(unlock) + return b.resetForProfileChangeLocked() } 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 49cfc3e07..60b5b2c5b 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.setPrefsLockedOnEntry(newp, unlock) + b.setPrefsLocked(newp) } type peerOptFunc func(*tailcfg.Node) diff --git a/ipn/ipnlocal/profiles.go b/ipn/ipnlocal/profiles.go index 1d312cfa6..7519ee157 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.resetForProfileChangeLockedOnEntry] after the netmap reset. + // [LocalBackend.resetForProfileChangeLocked] 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.resetForProfileChangeLockedOnEntry] 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.resetForProfileChangeLocked] 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,10 +494,9 @@ 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.setPrefsLockedOnEntry] + // 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] // or at least using [profileManager.SetPrefs]. // // While we should definitely clean this up to improve