From 55b1a4de74d72e3c6a25f82dab1631f2060281ef Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Mon, 13 Jul 2026 21:53:49 +0000 Subject: [PATCH] net/netcheck: don't mutate Client.TimeNow in AddReportHistoryForTest AddReportHistoryForTest temporarily swapped out Client.TimeNow without holding any lock, racing with concurrent GetReport calls reading it from ReSTUN goroutines during tests. Instead, pass the current time to addReportHistoryAndSetPreferredDERP explicitly so the test helper never needs to touch the field. Fixes #20438 Signed-off-by: Brad Fitzpatrick Change-Id: I9c4f0a2f9427b5f1d3e8b06a49f0d2b71c3ee8a4 --- net/netcheck/netcheck.go | 9 +++------ net/netcheck/netcheck_test.go | 4 ++-- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/net/netcheck/netcheck.go b/net/netcheck/netcheck.go index 5b6fb119f..8161ae48d 100644 --- a/net/netcheck/netcheck.go +++ b/net/netcheck/netcheck.go @@ -1026,7 +1026,7 @@ func (c *Client) finishAndStoreReport(rs *reportState, dm *tailcfg.DERPMap) *Rep report := rs.report.Clone() rs.mu.Unlock() - c.addReportHistoryAndSetPreferredDERP(rs, report, dm.View()) + c.addReportHistoryAndSetPreferredDERP(rs, report, dm.View(), c.timeNow()) c.logConciseReport(report, dm) return report @@ -1381,7 +1381,7 @@ func (c *Client) addReportAndPruneExpired(now time.Time, r *Report) { // addReportHistoryAndSetPreferredDERP adds r to the set of recent Reports // and mutates r.PreferredDERP to contain the best recent one. -func (c *Client) addReportHistoryAndSetPreferredDERP(rs *reportState, r *Report, dm tailcfg.DERPMapView) { +func (c *Client) addReportHistoryAndSetPreferredDERP(rs *reportState, r *Report, dm tailcfg.DERPMapView, now time.Time) { c.mu.Lock() defer c.mu.Unlock() @@ -1392,7 +1392,6 @@ func (c *Client) addReportHistoryAndSetPreferredDERP(rs *reportState, r *Report, // Add report to history, enforce retention window, then take the best (lowest) // latency seen per region across what remains. - now := c.timeNow() c.addReportAndPruneExpired(now, r) bestRecent := c.bestRecentLatencyLocked() @@ -1517,10 +1516,8 @@ func (c *Client) RecentRegionLatency() map[int]time.Duration { // r.PreferredDERP from that history. func (c *Client) AddReportHistoryForTest(dm *tailcfg.DERPMap, r *Report, now time.Time) { testenv.AssertInTest() - defer func(prev func() time.Time) { c.TimeNow = prev }(c.TimeNow) - c.TimeNow = func() time.Time { return now } rs := &reportState{c: c, start: now} - c.addReportHistoryAndSetPreferredDERP(rs, r, dm.View()) + c.addReportHistoryAndSetPreferredDERP(rs, r, dm.View(), now) } func updateLatency(m map[int]time.Duration, regionID int, d time.Duration) { diff --git a/net/netcheck/netcheck_test.go b/net/netcheck/netcheck_test.go index ea1e24040..70c07aacd 100644 --- a/net/netcheck/netcheck_test.go +++ b/net/netcheck/netcheck_test.go @@ -468,7 +468,7 @@ func TestAddReportHistoryAndSetPreferredDERP(t *testing.T) { for _, s := range tt.steps { fakeTime = fakeTime.Add(s.after) rs.start = fakeTime.Add(-100 * time.Millisecond) - c.addReportHistoryAndSetPreferredDERP(rs, s.r, dm.View()) + c.addReportHistoryAndSetPreferredDERP(rs, s.r, dm.View(), fakeTime) } lastReport := tt.steps[len(tt.steps)-1].r if got, want := len(c.prev), tt.wantPrevLen; got != want { @@ -522,7 +522,7 @@ func TestRecentReportsRetainFullNetcheck(t *testing.T) { regions = allRegions lastFull = now } - c.addReportHistoryAndSetPreferredDERP(&reportState{c: c, start: now}, mkReport(regions), dm.View()) + c.addReportHistoryAndSetPreferredDERP(&reportState{c: c, start: now}, mkReport(regions), dm.View(), now) // Recent latency must always cover every region, which is only // possible while a full report remains in c.prev.