Skip to content

Commit fc07039

Browse files
prattmicheschi
authored andcommitted
[release-branch.go1.17] runtime: add race annotations to metricsSema
metricsSema protects the metrics map. The map implementation is race instrumented regardless of which package is it called from. semacquire/semrelease are not automatically race instrumented, so we can trigger race false positives without manually annotating our lock acquire and release. See similar instrumentation on trace.shutdownSema and reflectOffs.lock. Fixes #53589. For #53542. Change-Id: Ia3fd239ac860e037d09c7cb9c4ad267391e70705 Reviewed-on: https://go-review.googlesource.com/c/go/+/414517 Run-TryBot: Michael Pratt <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Cherry Mui <[email protected]> (cherry picked from commit d6481d5) Reviewed-on: https://go-review.googlesource.com/c/go/+/415196
1 parent 9ef614f commit fc07039

File tree

2 files changed

+27
-10
lines changed

2 files changed

+27
-10
lines changed

src/runtime/export_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -321,9 +321,9 @@ func ReadMetricsSlow(memStats *MemStats, samplesp unsafe.Pointer, len, cap int)
321321

322322
// Initialize the metrics beforehand because this could
323323
// allocate and skew the stats.
324-
semacquire(&metricsSema)
324+
metricsLock()
325325
initMetrics()
326-
semrelease(&metricsSema)
326+
metricsUnlock()
327327

328328
systemstack(func() {
329329
// Read memstats first. It's going to flush

src/runtime/metrics.go

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,12 @@ import (
1212
)
1313

1414
var (
15-
// metrics is a map of runtime/metrics keys to
16-
// data used by the runtime to sample each metric's
17-
// value.
15+
// metrics is a map of runtime/metrics keys to data used by the runtime
16+
// to sample each metric's value. metricsInit indicates it has been
17+
// initialized.
18+
//
19+
// These fields are protected by metricsSema which should be
20+
// locked/unlocked with metricsLock() / metricsUnlock().
1821
metricsSema uint32 = 1
1922
metricsInit bool
2023
metrics map[string]metricData
@@ -34,6 +37,23 @@ type metricData struct {
3437
compute func(in *statAggregate, out *metricValue)
3538
}
3639

40+
func metricsLock() {
41+
// Acquire the metricsSema but with handoff. Operations are typically
42+
// expensive enough that queueing up goroutines and handing off between
43+
// them will be noticeably better-behaved.
44+
semacquire1(&metricsSema, true, 0, 0)
45+
if raceenabled {
46+
raceacquire(unsafe.Pointer(&metricsSema))
47+
}
48+
}
49+
50+
func metricsUnlock() {
51+
if raceenabled {
52+
racerelease(unsafe.Pointer(&metricsSema))
53+
}
54+
semrelease(&metricsSema)
55+
}
56+
3757
// initMetrics initializes the metrics map if it hasn't been yet.
3858
//
3959
// metricsSema must be held.
@@ -546,10 +566,7 @@ func readMetrics(samplesp unsafe.Pointer, len int, cap int) {
546566
sl := slice{samplesp, len, cap}
547567
samples := *(*[]metricSample)(unsafe.Pointer(&sl))
548568

549-
// Acquire the metricsSema but with handoff. This operation
550-
// is expensive enough that queueing up goroutines and handing
551-
// off between them will be noticeably better-behaved.
552-
semacquire1(&metricsSema, true, 0, 0)
569+
metricsLock()
553570

554571
// Ensure the map is initialized.
555572
initMetrics()
@@ -573,5 +590,5 @@ func readMetrics(samplesp unsafe.Pointer, len int, cap int) {
573590
data.compute(&agg, &sample.value)
574591
}
575592

576-
semrelease(&metricsSema)
593+
metricsUnlock()
577594
}

0 commit comments

Comments
 (0)