From f4094c301f6bd1d4b69df4ec0971348b5de286ba Mon Sep 17 00:00:00 2001 From: James Sanderson Date: Fri, 3 Oct 2025 23:47:35 +0100 Subject: [PATCH] WIP: alternative option for fixing race condition in requestEngineStatusAndWaitForStopped --- ipn/ipnlocal/local.go | 29 +++++++++++------------------ 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 54897b9ff..20eda055e 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -2200,12 +2200,12 @@ func (b *LocalBackend) resolveExitNodeIPLocked(prefs *ipn.Prefs) (prefsChanged b func (b *LocalBackend) setWgengineStatus(s *wgengine.Status, err error) { if err != nil { b.logf("wgengine status error: %v", err) - b.broadcastStatusChanged() + b.maybeBroadcastStatusChanged(nil) return } if s == nil { b.logf("[unexpected] non-error wgengine update with status=nil: %v", s) - b.broadcastStatusChanged() + b.maybeBroadcastStatusChanged(nil) return } @@ -2214,6 +2214,7 @@ func (b *LocalBackend) setWgengineStatus(s *wgengine.Status, err error) { // Don't process a status update that is older than the one we have // already processed. (corp#2579) b.mu.Unlock() + b.maybeBroadcastStatusChanged(s) return } b.lastStatusTime = s.AsOf @@ -2232,12 +2233,15 @@ func (b *LocalBackend) setWgengineStatus(s *wgengine.Status, err error) { } b.stateMachine() } - b.broadcastStatusChanged() + b.maybeBroadcastStatusChanged(s) b.send(ipn.Notify{Engine: &es}) } -// broadcastStatusChanged must not be called with b.mu held. -func (b *LocalBackend) broadcastStatusChanged() { +// maybeBroadcastStatusChanged must not be called with b.mu held. +func (b *LocalBackend) maybeBroadcastStatusChanged(s *wgengine.Status) { + if s != nil && (s.DERPs != 0 || len(s.Peers) != 0) { + return + } // The sync.Cond docs say: "It is allowed but not required for the caller to hold c.L during the call." // In this particular case, we must acquire b.mu. Otherwise we might broadcast before // the waiter (in requestEngineStatusAndWait) starts to wait, in which case @@ -5696,19 +5700,8 @@ func (b *LocalBackend) requestEngineStatusAndWaitForStopped() { b.goTracker.Go(b.e.RequestStatus) b.logf("requestEngineStatusAndWaitForStopped: waiting...") - for { - b.statusChanged.Wait() // temporarily releases lock while waiting - - if !b.blocked { - b.logf("requestEngineStatusAndWaitForStopped: engine is no longer blocked, must have stopped and started again, not safe to wait.") - break - } - if b.engineStatus.NumLive == 0 && b.engineStatus.LiveDERPs == 0 { - b.logf("requestEngineStatusAndWaitForStopped: engine is stopped.") - break - } - b.logf("requestEngineStatusAndWaitForStopped: engine is still running. Waiting...") - } + b.statusChanged.Wait() // temporarily releases lock while waiting + b.logf("requestEngineStatusAndWaitForStopped: engine was stopped.") } // setControlClientLocked sets the control client to cc,