net/netcheck: fix race condition initializting RegionLatency maps.

Under some conditions, code would try to look things up in the maps
before the first call to updateLatency. I don't see any reason to delay
initialization of the maps, so let's just init them right away when
creating the Report instance.

Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>
This commit is contained in:
Avery Pennarun 2020-05-28 03:37:46 -04:00
parent 5eb09c8f5e
commit 7cd9ff3dde
2 changed files with 14 additions and 10 deletions

View File

@ -518,17 +518,17 @@ func (rs *reportState) addNodeLatency(node *tailcfg.DERPNode, ipp netaddr.IPPort
ret := rs.report ret := rs.report
ret.UDP = true ret.UDP = true
updateLatency(&ret.RegionLatency, node.RegionID, d) updateLatency(ret.RegionLatency, node.RegionID, d)
switch { switch {
case ipp.IP.Is6(): case ipp.IP.Is6():
updateLatency(&ret.RegionV6Latency, node.RegionID, d) updateLatency(ret.RegionV6Latency, node.RegionID, d)
ret.IPv6 = true ret.IPv6 = true
ret.GlobalV6 = ipPortStr ret.GlobalV6 = ipPortStr
// TODO: track MappingVariesByDestIP for IPv6 // TODO: track MappingVariesByDestIP for IPv6
// too? Would be sad if so, but who knows. // too? Would be sad if so, but who knows.
case ipp.IP.Is4(): case ipp.IP.Is4():
updateLatency(&ret.RegionV4Latency, node.RegionID, d) updateLatency(ret.RegionV4Latency, node.RegionID, d)
if rs.gotEP4 == "" { if rs.gotEP4 == "" {
rs.gotEP4 = ipPortStr rs.gotEP4 = ipPortStr
ret.GlobalV4 = ipPortStr ret.GlobalV4 = ipPortStr
@ -543,6 +543,14 @@ func (rs *reportState) addNodeLatency(node *tailcfg.DERPNode, ipp netaddr.IPPort
} }
} }
func newReport() *Report {
return &Report{
RegionLatency: make(map[int]time.Duration),
RegionV4Latency: make(map[int]time.Duration),
RegionV6Latency: make(map[int]time.Duration),
}
}
// GetReport gets a report. // GetReport gets a report.
// //
// It may not be called concurrently with itself. // It may not be called concurrently with itself.
@ -564,7 +572,7 @@ func (c *Client) GetReport(ctx context.Context, dm *tailcfg.DERPMap) (*Report, e
} }
rs := &reportState{ rs := &reportState{
c: c, c: c,
report: new(Report), report: newReport(),
inFlight: map[stun.TxID]func(netaddr.IPPort){}, inFlight: map[stun.TxID]func(netaddr.IPPort){},
hairTX: stun.NewTxID(), // random payload hairTX: stun.NewTxID(), // random payload
gotHairSTUN: make(chan *net.UDPAddr, 1), gotHairSTUN: make(chan *net.UDPAddr, 1),
@ -806,11 +814,7 @@ func (c *Client) addReportHistoryAndSetPreferredDERP(r *Report) {
} }
} }
func updateLatency(mp *map[int]time.Duration, regionID int, d time.Duration) { func updateLatency(m map[int]time.Duration, regionID int, d time.Duration) {
if *mp == nil {
*mp = make(map[int]time.Duration)
}
m := *mp
if prev, ok := m[regionID]; !ok || d < prev { if prev, ok := m[regionID]; !ok || d < prev {
m[regionID] = d m[regionID] = d
} }

View File

@ -96,7 +96,7 @@ func TestWorksWhenUDPBlocked(t *testing.T) {
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
want := new(Report) want := newReport()
if !reflect.DeepEqual(r, want) { if !reflect.DeepEqual(r, want) {
t.Errorf("mismatch\n got: %+v\nwant: %+v\n", r, want) t.Errorf("mismatch\n got: %+v\nwant: %+v\n", r, want)