aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMichael Pratt <mpratt@google.com>2022-06-28 15:17:12 -0400
committerHeschi Kreinick <heschi@google.com>2022-07-06 19:34:44 +0000
commitfc07039e2339a80f53e7db5e214b5be504bc1df6 (patch)
tree6b334693dbd8406fd2e5c8b562683218cf874453
parent9ef614f5aa03ed30664c982c706c98c8b78c99bf (diff)
downloadgo-fc07039e2339a80f53e7db5e214b5be504bc1df6.tar.gz
go-fc07039e2339a80f53e7db5e214b5be504bc1df6.zip
[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 <mpratt@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> (cherry picked from commit d6481d5b9662b29453004204746945a93a6b4eb2) Reviewed-on: https://go-review.googlesource.com/c/go/+/415196
-rw-r--r--src/runtime/export_test.go4
-rw-r--r--src/runtime/metrics.go33
2 files changed, 27 insertions, 10 deletions
diff --git a/src/runtime/export_test.go b/src/runtime/export_test.go
index 03ccfe10a8..3cee4dec57 100644
--- a/src/runtime/export_test.go
+++ b/src/runtime/export_test.go
@@ -321,9 +321,9 @@ func ReadMetricsSlow(memStats *MemStats, samplesp unsafe.Pointer, len, cap int)
// Initialize the metrics beforehand because this could
// allocate and skew the stats.
- semacquire(&metricsSema)
+ metricsLock()
initMetrics()
- semrelease(&metricsSema)
+ metricsUnlock()
systemstack(func() {
// Read memstats first. It's going to flush
diff --git a/src/runtime/metrics.go b/src/runtime/metrics.go
index 4af2b0aef9..922dd2f814 100644
--- a/src/runtime/metrics.go
+++ b/src/runtime/metrics.go
@@ -12,9 +12,12 @@ import (
)
var (
- // metrics is a map of runtime/metrics keys to
- // data used by the runtime to sample each metric's
- // value.
+ // metrics is a map of runtime/metrics keys to data used by the runtime
+ // to sample each metric's value. metricsInit indicates it has been
+ // initialized.
+ //
+ // These fields are protected by metricsSema which should be
+ // locked/unlocked with metricsLock() / metricsUnlock().
metricsSema uint32 = 1
metricsInit bool
metrics map[string]metricData
@@ -34,6 +37,23 @@ type metricData struct {
compute func(in *statAggregate, out *metricValue)
}
+func metricsLock() {
+ // Acquire the metricsSema but with handoff. Operations are typically
+ // expensive enough that queueing up goroutines and handing off between
+ // them will be noticeably better-behaved.
+ semacquire1(&metricsSema, true, 0, 0)
+ if raceenabled {
+ raceacquire(unsafe.Pointer(&metricsSema))
+ }
+}
+
+func metricsUnlock() {
+ if raceenabled {
+ racerelease(unsafe.Pointer(&metricsSema))
+ }
+ semrelease(&metricsSema)
+}
+
// initMetrics initializes the metrics map if it hasn't been yet.
//
// metricsSema must be held.
@@ -546,10 +566,7 @@ func readMetrics(samplesp unsafe.Pointer, len int, cap int) {
sl := slice{samplesp, len, cap}
samples := *(*[]metricSample)(unsafe.Pointer(&sl))
- // Acquire the metricsSema but with handoff. This operation
- // is expensive enough that queueing up goroutines and handing
- // off between them will be noticeably better-behaved.
- semacquire1(&metricsSema, true, 0, 0)
+ metricsLock()
// Ensure the map is initialized.
initMetrics()
@@ -573,5 +590,5 @@ func readMetrics(samplesp unsafe.Pointer, len int, cap int) {
data.compute(&agg, &sample.value)
}
- semrelease(&metricsSema)
+ metricsUnlock()
}