diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index e5fafb5bd..51f926560 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -950,8 +950,12 @@ func (b *LocalBackend) pauseOrResumeControlClientLocked() { // down, clients switch over to other replicas whilst the existing connections are kept alive for some period of time. func (b *LocalBackend) DisconnectControl() { b.mu.Lock() - defer b.mu.Unlock() - b.resetControlClientLocked() + cc := b.resetControlClientLocked() + b.mu.Unlock() + + if cc != nil { + cc.Shutdown() + } } // linkChange is our network monitor callback, called whenever the network changes. @@ -2417,6 +2421,14 @@ func (b *LocalBackend) startLocked(opts ipn.Options) error { logf := logger.WithPrefix(b.logf, "Start: ") b.startOnce.Do(b.initOnce) + var clientToShutdown controlclient.Client + defer func() { + if clientToShutdown != nil { + // Shutdown outside of b.mu to avoid deadlocks. + b.goTracker.Go(clientToShutdown.Shutdown) + } + }() + if opts.UpdatePrefs != nil { if err := b.checkPrefsLocked(opts.UpdatePrefs); err != nil { return err @@ -2459,7 +2471,7 @@ func (b *LocalBackend) startLocked(opts ipn.Options) error { // into sync with the minimal changes. But that's not how it // is right now, which is a sign that the code is still too // complicated. - b.resetControlClientLocked() + clientToShutdown = b.resetControlClientLocked() httpTestClient := b.httpTestClient if b.hostinfo != nil { @@ -5812,12 +5824,13 @@ func (b *LocalBackend) setControlClientLocked(cc controlclient.Client) { b.ignoreControlClientUpdates.Store(cc == nil) } -// resetControlClientLocked sets b.cc to nil and shuts down the previous -// control client, if any. -func (b *LocalBackend) resetControlClientLocked() { +// resetControlClientLocked sets b.cc to nil and returns the old value. If the +// returned value is non-nil, the caller must call Shutdown on it after +// releasing b.mu. +func (b *LocalBackend) resetControlClientLocked() controlclient.Client { syncs.RequiresMutex(&b.mu) if b.cc == nil { - return + return nil } b.resetAuthURLLocked() @@ -5837,7 +5850,7 @@ func (b *LocalBackend) resetControlClientLocked() { } prev := b.cc b.setControlClientLocked(nil) - prev.Shutdown() + return prev } // resetAuthURLLocked resets authURL, canceling any pending interactive login. @@ -6931,7 +6944,10 @@ func (b *LocalBackend) resetForProfileChangeLocked() error { b.updateFilterLocked(ipn.PrefsView{}) // Reset the NetworkMap in the engine b.e.SetNetworkMap(new(netmap.NetworkMap)) - b.resetControlClientLocked() + if prevCC := b.resetControlClientLocked(); prevCC != nil { + // Shutdown outside of b.mu to avoid deadlocks. + b.goTracker.Go(prevCC.Shutdown) + } // TKA errors should not prevent resetting the backend state. // However, we should still return the error to the caller. tkaErr := b.initTKALocked() @@ -7010,7 +7026,10 @@ func (b *LocalBackend) ResetAuth() error { b.mu.Lock() defer b.mu.Unlock() - b.resetControlClientLocked() + if prevCC := b.resetControlClientLocked(); prevCC != nil { + // Shutdown outside of b.mu to avoid deadlocks. + b.goTracker.Go(prevCC.Shutdown) + } if err := b.clearMachineKeyLocked(); err != nil { return err }