[Recheck watcher ctx cancellation under conn.mu in onWGDisconnected

onWGDisconnected only checked conn.ctx (the engine-scoped context), never
the watcher's own context. disableWgWatcherIfNeeded cancels the wgWatcherCtx,
not conn.ctx, so a disabled watcher's timeout callback did not see the
cancellation.

handshakeCheck runs lock-free, so between the ctx check in periodicHandshakeCheck
and acquiring conn.mu a fast disconnect/reconnect can slip in: the stale watcher
then acquires the lock and tears down the *new*, healthy connection based on the
old timeout, forcing the guard into an unnecessary reconnect (flap).

Recheck watcherCtx.Err() under conn.mu so a superseded watcher exits without
touching the connection that replaced it.
This commit is contained in:
riccardom
2026-07-03 11:22:53 +02:00
parent 60104e000b
commit ec98c930cb

View File

@@ -662,11 +662,16 @@ func (conn *Conn) onGuardEvent() {
}
}
func (conn *Conn) onWGDisconnected() {
// onWGDisconnected is invoked by the watcher goroutine when a handshake timeout is detected.
// watcherCtx is the context of the watcher that fired: the timeout check runs lock-free, so by
// the time we acquire conn.mu the watcher may have been cancelled (disabled) and a new connection
// (and watcher) may already be in place. Re-checking watcherCtx under the lock prevents a stale
// watcher from tearing down the connection that superseded it.
func (conn *Conn) onWGDisconnected(watcherCtx context.Context) {
conn.mu.Lock()
defer conn.mu.Unlock()
if conn.ctx.Err() != nil {
if conn.ctx.Err() != nil || watcherCtx.Err() != nil {
return
}
@@ -822,7 +827,8 @@ func (conn *Conn) enableWgWatcherIfNeeded(enabledTime time.Time) {
conn.wgWatcherWg.Add(1)
go func() {
defer conn.wgWatcherWg.Done()
watcher.EnableWgWatcher(wgWatcherCtx, enabledTime, conn.onWGDisconnected, conn.onWGHandshakeSuccess)
onDisconnected := func() { conn.onWGDisconnected(wgWatcherCtx) }
watcher.EnableWgWatcher(wgWatcherCtx, enabledTime, onDisconnected, conn.onWGHandshakeSuccess)
}()
}