From 926aec56e89359839e52bce7f55b81c3b8aed255 Mon Sep 17 00:00:00 2001 From: Nick Khyl Date: Mon, 21 Apr 2025 11:38:55 -0500 Subject: [PATCH] ipn/ipnlocal: move resetDialPlan to after cc.Logout() returns We shouldn't be resetting the current dial plan until cc.Logout() has returned, so we move the resetDialPlan() call accordingly. We also update a comment that become inaccurate sometime since 2020. Finally, we add a resetDialPlan() call when the current profile is deleted. It results in a switch to a new, empty profile and resets the state machine, so the dial plan should be reset as well. Updates #cleanup Updates #11938 Signed-off-by: Nick Khyl --- ipn/ipnlocal/local.go | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index ef5ec267f..ee5a99edf 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -5962,17 +5962,14 @@ func (b *LocalBackend) Logout(ctx context.Context) error { } // b.mu is now unlocked, after editPrefsLockedOnEntry. - // Clear any previous dial plan(s), if set. - b.resetDialPlan() - if cc == nil { - // Double Logout can happen via repeated IPN - // connections to ipnserver making it repeatedly - // transition from 1->0 total connections, which on - // Windows by default ("client mode") causes a Logout - // on the transition to zero. // Previously this crashed when we asserted that c was non-nil // here. + // It can be nil due to atypical locking patterns in LocalBackend. + // For example, the old cc can be set to nil while holding b.mu, + // then b.mu is unlocked before calling [LocalBackend.Start], + // which re-acquires the lock and sets a new cc. + // If Logout is called in between, cc can be nil. return errors.New("no controlclient") } @@ -5983,6 +5980,9 @@ func (b *LocalBackend) Logout(ctx context.Context) error { unlock = b.lockAndGetUnlock() defer unlock() + // Clear any previous dial plan(s), if set. + b.resetDialPlan() + if err := b.pm.DeleteProfile(profile.ID()); err != nil { b.logf("error deleting profile: %v", err) return err @@ -7334,6 +7334,9 @@ func (b *LocalBackend) DeleteProfile(p ipn.ProfileID) error { if !needToRestart { return nil } + // Deleting the current profile switches to a new, empty one. + // Its ControlURL hasn't been set yet, so we conservatively reset the dialPlan. + b.resetDialPlan() return b.resetForProfileChangeLockedOnEntry(unlock) }