From 825b7c479f5ab4123528a90ae744b8a3f0859821 Mon Sep 17 00:00:00 2001 From: Simon Law Date: Mon, 29 Jun 2026 09:38:07 -0700 Subject: [PATCH] wgengine/magicsock: fix data race in TestNetworkSendErrors (#20261) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `TestNetworkSendErrors/network-down` causes a data race because it tried to `tstest.Replace` the `checkNetworkDownDuringTests` global while `wgengine.Conn.networkDown` would read from it. This patch moves this flag into a field within the `wgengine.Conn` struct, so there’s no chance that two tests could trample on each other. It also renames this field to `Conn.checkNetworkUpDuringTests`, because `Conn.networkUp` is the name of the field that gets checked. Fixes #20260 Signed-off-by: Simon Law --- wgengine/magicsock/magicsock.go | 13 ++++++++----- wgengine/magicsock/magicsock_test.go | 3 +-- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index d33db5f8f..cf2639eb0 100644 --- a/wgengine/magicsock/magicsock.go +++ b/wgengine/magicsock/magicsock.go @@ -425,6 +425,13 @@ type Conn struct { // homeDERPGauge is the usermetric gauge for the home DERP region ID. // This can be nil when [Options.Metrics] are not enabled. homeDERPGauge *usermetric.Gauge + + // checkNetworkUpDuringTests controls whether [Conn.networkDown] + // will report the value of [Conn.networkUp] while running tests. + // + // This allows tests to pass when the user's machine is offline, + // but allows us to still test network-down behaviour when desired. + checkNetworkUpDuringTests bool } // SetDebugLoggingEnabled controls whether spammy debug logging is enabled. @@ -1482,14 +1489,10 @@ func (c *Conn) LocalPort() uint16 { var errNetworkDown = errors.New("magicsock: network down") -// This allows tests to pass when the user's machine is offline, but allows us -// to still test network-down behaviour when desired. -var checkNetworkDownDuringTests = false - func (c *Conn) networkDown() bool { // For tests, always assume the network is up unless we're explicitly // testing this behaviour. - if envknob.AssumeNetworkUp() || (testenv.InTest() && !checkNetworkDownDuringTests) { + if envknob.AssumeNetworkUp() || (testenv.InTest() && !c.checkNetworkUpDuringTests) { return false } return !c.networkUp.Load() diff --git a/wgengine/magicsock/magicsock_test.go b/wgengine/magicsock/magicsock_test.go index 8797eab76..30d6c2910 100644 --- a/wgengine/magicsock/magicsock_test.go +++ b/wgengine/magicsock/magicsock_test.go @@ -3386,9 +3386,8 @@ func TestNetworkSendErrors(t *testing.T) { t.Skipf("skipping on %s", runtime.GOOS) } - tstest.Replace(t, &checkNetworkDownDuringTests, true) - conn, reg := newTestConnAndRegistry(t) + conn.checkNetworkUpDuringTests = true buffs := [][]byte{{00, 00, 00, 00, 00, 00, 00, 00}} ep := &lazyEndpoint{