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 <bradfitz@tailscale.com> Change-Id: I9c4f0a2f9427b5f1d3e8b06a49f0d2b71c3ee8a4
This commit is contained in:
committed by
Brad Fitzpatrick
parent
b803ba048c
commit
55b1a4de74
@@ -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) {
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user