From fa49009eee1714d56f3307b92cd6f634d0e1e4e4 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Thu, 14 May 2026 20:54:10 +0000 Subject: [PATCH] wgengine: simplify ResetAndStop, drop drain loop Since f343b496c3 ("wgengine, all: remove LazyWG, use wireguard-go callback API for on-demand peers"), Reconfig is fully synchronous: magicConn.UpdatePeers, wgdev.RemovePeer, router.Set, and dns.Set all return when the work is done, and the peer list is updated under wgLock before Reconfig returns. So after Reconfig with empty configs, len(st.Peers) is already 0. The old loop also waited for st.DERPs to drain to 0, but UpdatePeers only edits maps; active DERP connections idle out on their own timeout. The sole caller (LocalBackend.stopEngineAndWait) doesn't inspect st.DERPs anyway; it just hands the Status to setWgengineStatusLocked. So the drain-wait was for nothing observable and could theoretically (or at least appear to readers to) loop forever holding b.mu. Remove that reader confusion by removing the backoff loop entirely. Updates #19759 Change-Id: Ibfac3f0baabcad7604b713c934a8fc37932e0a50 Signed-off-by: Brad Fitzpatrick --- wgengine/userspace.go | 21 ++++----------------- 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/wgengine/userspace.go b/wgengine/userspace.go index 23edf30b3..222df1bc8 100644 --- a/wgengine/userspace.go +++ b/wgengine/userspace.go @@ -47,7 +47,6 @@ import ( "tailscale.com/types/logger" "tailscale.com/types/netmap" "tailscale.com/types/views" - "tailscale.com/util/backoff" "tailscale.com/util/checkchange" "tailscale.com/util/clientmetric" "tailscale.com/util/eventbus" @@ -749,29 +748,17 @@ func hasOverlap(aips, rips views.Slice[netip.Prefix]) bool { } // ResetAndStop resets the engine to a clean state (like calling Reconfig -// with all pointers to zero values) and waits for it to be fully stopped, -// with no live peers or DERPs. +// with all pointers to zero values) and returns the resulting status. // // Unlike Reconfig, it does not return ErrNoChanges. // -// If the engine stops, returns the status. NB that this status will not be sent -// to the registered status callback, it is on the caller to ensure this status -// is handled appropriately. +// The returned status will not be sent to the registered status callback; +// it is on the caller to ensure this status is handled appropriately. func (e *userspaceEngine) ResetAndStop() (*Status, error) { if err := e.Reconfig(&wgcfg.Config{}, &router.Config{}, &dns.Config{}); err != nil && !errors.Is(err, ErrNoChanges) { return nil, err } - bo := backoff.NewBackoff("UserspaceEngineResetAndStop", e.logf, 1*time.Second) - for { - st, err := e.getStatus() - if err != nil { - return nil, err - } - if len(st.Peers) == 0 && st.DERPs == 0 { - return st, nil - } - bo.BackOff(context.Background(), fmt.Errorf("waiting for engine to stop: peers=%d derps=%d", len(st.Peers), st.DERPs)) - } + return e.getStatus() } func (e *userspaceEngine) PatchDiscoKey(pub key.NodePublic, disco key.DiscoPublic) {