wgengine/magicsock: fix data race in TestNetworkSendErrors (#20261)
`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 <sfllaw@tailscale.com>
This commit is contained in:
@@ -425,6 +425,13 @@ type Conn struct {
|
|||||||
// homeDERPGauge is the usermetric gauge for the home DERP region ID.
|
// homeDERPGauge is the usermetric gauge for the home DERP region ID.
|
||||||
// This can be nil when [Options.Metrics] are not enabled.
|
// This can be nil when [Options.Metrics] are not enabled.
|
||||||
homeDERPGauge *usermetric.Gauge
|
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.
|
// SetDebugLoggingEnabled controls whether spammy debug logging is enabled.
|
||||||
@@ -1482,14 +1489,10 @@ func (c *Conn) LocalPort() uint16 {
|
|||||||
|
|
||||||
var errNetworkDown = errors.New("magicsock: network down")
|
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 {
|
func (c *Conn) networkDown() bool {
|
||||||
// For tests, always assume the network is up unless we're explicitly
|
// For tests, always assume the network is up unless we're explicitly
|
||||||
// testing this behaviour.
|
// testing this behaviour.
|
||||||
if envknob.AssumeNetworkUp() || (testenv.InTest() && !checkNetworkDownDuringTests) {
|
if envknob.AssumeNetworkUp() || (testenv.InTest() && !c.checkNetworkUpDuringTests) {
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
return !c.networkUp.Load()
|
return !c.networkUp.Load()
|
||||||
|
|||||||
@@ -3386,9 +3386,8 @@ func TestNetworkSendErrors(t *testing.T) {
|
|||||||
t.Skipf("skipping on %s", runtime.GOOS)
|
t.Skipf("skipping on %s", runtime.GOOS)
|
||||||
}
|
}
|
||||||
|
|
||||||
tstest.Replace(t, &checkNetworkDownDuringTests, true)
|
|
||||||
|
|
||||||
conn, reg := newTestConnAndRegistry(t)
|
conn, reg := newTestConnAndRegistry(t)
|
||||||
|
conn.checkNetworkUpDuringTests = true
|
||||||
|
|
||||||
buffs := [][]byte{{00, 00, 00, 00, 00, 00, 00, 00}}
|
buffs := [][]byte{{00, 00, 00, 00, 00, 00, 00, 00}}
|
||||||
ep := &lazyEndpoint{
|
ep := &lazyEndpoint{
|
||||||
|
|||||||
Reference in New Issue
Block a user