]> Cypherpunks repositories - gostls13.git/commitdiff
runtime: add race annotations to metricsSema
authorMichael Pratt <mpratt@google.com>
Tue, 28 Jun 2022 19:17:12 +0000 (15:17 -0400)
committerDavid Chase <drchase@google.com>
Wed, 29 Jun 2022 16:30:19 +0000 (16:30 +0000)
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 #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>
src/runtime/export_test.go
src/runtime/metrics.go

index 1626f99336f9eb15c2aea079130483ee2fc6dbd6..9639946fa9c5010ec32d8b96a53adb4eac5f4261 100644 (file)
@@ -312,9 +312,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
index 75ad318b07bc4b0ab62ad46cb12a841e4d7cf949..986121b9c2f976a68b25586fb01e29836d2adbcd 100644 (file)
@@ -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.
@@ -570,10 +590,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()
@@ -597,5 +614,5 @@ func readMetrics(samplesp unsafe.Pointer, len int, cap int) {
                data.compute(&agg, &sample.value)
        }
 
-       semrelease(&metricsSema)
+       metricsUnlock()
 }