ipn/ipnlocal: do not call controlclient.Client.Shutdown with b.mu held

This fixes a regression in #17804 that caused a deadlock.

Updates #18052

Signed-off-by: Nick Khyl <nickk@tailscale.com>
This commit is contained in:
Nick Khyl 2025-11-25 08:58:36 -06:00 committed by Nick Khyl
parent d4821cdc2f
commit 7073f246d3

View File

@ -944,12 +944,12 @@ func (b *LocalBackend) pauseOrResumeControlClientLocked() {
// down, clients switch over to other replicas whilst the existing connections are kept alive for some period of time. // down, clients switch over to other replicas whilst the existing connections are kept alive for some period of time.
func (b *LocalBackend) DisconnectControl() { func (b *LocalBackend) DisconnectControl() {
b.mu.Lock() b.mu.Lock()
defer b.mu.Unlock()
cc := b.resetControlClientLocked() cc := b.resetControlClientLocked()
if cc == nil { b.mu.Unlock()
return
if cc != nil {
cc.Shutdown()
} }
cc.Shutdown()
} }
// linkChange is our network monitor callback, called whenever the network changes. // linkChange is our network monitor callback, called whenever the network changes.
@ -2408,7 +2408,8 @@ func (b *LocalBackend) startLocked(opts ipn.Options) error {
var clientToShutdown controlclient.Client var clientToShutdown controlclient.Client
defer func() { defer func() {
if clientToShutdown != nil { if clientToShutdown != nil {
clientToShutdown.Shutdown() // Shutdown outside of b.mu to avoid deadlocks.
b.goTracker.Go(clientToShutdown.Shutdown)
} }
}() }()
@ -6891,7 +6892,8 @@ func (b *LocalBackend) resetForProfileChangeLocked() error {
// Reset the NetworkMap in the engine // Reset the NetworkMap in the engine
b.e.SetNetworkMap(new(netmap.NetworkMap)) b.e.SetNetworkMap(new(netmap.NetworkMap))
if prevCC := b.resetControlClientLocked(); prevCC != nil { if prevCC := b.resetControlClientLocked(); prevCC != nil {
defer prevCC.Shutdown() // Shutdown outside of b.mu to avoid deadlocks.
b.goTracker.Go(prevCC.Shutdown)
} }
// TKA errors should not prevent resetting the backend state. // TKA errors should not prevent resetting the backend state.
// However, we should still return the error to the caller. // However, we should still return the error to the caller.
@ -6972,7 +6974,8 @@ func (b *LocalBackend) ResetAuth() error {
defer b.mu.Unlock() defer b.mu.Unlock()
if prevCC := b.resetControlClientLocked(); prevCC != nil { if prevCC := b.resetControlClientLocked(); prevCC != nil {
defer prevCC.Shutdown() // Shutdown outside of b.mu to avoid deadlocks.
b.goTracker.Go(prevCC.Shutdown)
} }
if err := b.clearMachineKeyLocked(); err != nil { if err := b.clearMachineKeyLocked(); err != nil {
return err return err