From 4334dfa7d5ccbee1daf5acf30b33557bbca66525 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Claus=20Lensb=C3=B8l?= Date: Tue, 31 Mar 2026 12:47:13 -0400 Subject: [PATCH] control/controlclient: take mapsession and release lock early in sub (#19192) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The disco key subscriber could deadlock in a scenario where a self node update came through the control path into the mapSession after the disco key subscriber had taken the lock, but before it had pushed the netmap change, as both the subscriber and onSelfNodeChanged needs the controlclient lock. The subscriber can safely take the mapsession as the changequeue has its own lock for inserting records, and also checks if the queue has been closed before inserting. Updates #12639 Signed-off-by: Claus Lensbøl --- control/controlclient/direct.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go index 1569d7517..3b4e6ba9b 100644 --- a/control/controlclient/direct.go +++ b/control/controlclient/direct.go @@ -361,11 +361,16 @@ func NewDirect(opts Options) (*Direct, error) { c.controlTimePub = eventbus.Publish[ControlTime](c.busClient) discoKeyPub := eventbus.Publish[events.PeerDiscoKeyUpdate](c.busClient) eventbus.SubscribeFunc(c.busClient, func(update events.DiscoKeyAdvertisement) { - c.mu.Lock() - defer c.mu.Unlock() c.logf("controlclient direct: got TSMP disco key advertisement from %v via eventbus", update.Src) - if c.streamingMapSession != nil { - nm := c.streamingMapSession.netmap() + var nm *netmap.NetworkMap + c.mu.Lock() + sess := c.streamingMapSession + if sess != nil { + nm = c.streamingMapSession.netmap() + } + c.mu.Unlock() + + if sess != nil { peer, ok := nm.PeerByTailscaleIP(update.Src) if !ok { return @@ -375,7 +380,7 @@ func NewDirect(opts Options) (*Direct, error) { // If we update without error, return. If the err indicates that the // mapSession has gone away, we want to fall back to pushing the key // further down the chain. - if err := c.streamingMapSession.updateDiscoForNode( + if err := sess.updateDiscoForNode( peer.ID(), peer.Key(), update.Key, time.Now(), false); err == nil || !errors.Is(err, ErrChangeQueueClosed) { return