diff options
author | Michael Pratt <mpratt@google.com> | 2022-06-28 15:17:12 -0400 |
---|---|---|
committer | Heschi Kreinick <heschi@google.com> | 2022-07-06 19:34:44 +0000 |
commit | fc07039e2339a80f53e7db5e214b5be504bc1df6 (patch) | |
tree | 6b334693dbd8406fd2e5c8b562683218cf874453 /src | |
parent | 9ef614f5aa03ed30664c982c706c98c8b78c99bf (diff) | |
download | go-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
Diffstat (limited to 'src')
-rw-r--r-- | src/runtime/export_test.go | 4 | ||||
-rw-r--r-- | src/runtime/metrics.go | 33 |
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() } |