commit d6481d5b9662b29453004204746945a93a6b4eb2
author Michael Pratt <mpratt@google.com> Tue Jun 28 15:17:12 2022 -0400
committer David Chase <drchase@google.com> Wed Jun 29 16:30:19 2022 +0000
tree 188370350e876c226664389ebe1d4cfca5475d1b
parent bd1783e812bf10f5829b742135ef886623cccc5c

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 #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

diff --git a/src/runtime/export_test.go b/src/runtime/export_test.go
index 1626f99..9639946 100644
--- a/src/runtime/export_test.go
+++ b/src/runtime/export_test.go
@@ -312,9 +312,9 @@
 
 	// 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

src/runtime/metrics.go

diff --git a/src/runtime/metrics.go b/src/runtime/metrics.go
index 75ad318..986121b 100644
--- a/src/runtime/metrics.go
+++ b/src/runtime/metrics.go
@@ -12,9 +12,12 @@
 )
 
 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 @@
 	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 @@
 	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 @@
 		data.compute(&agg, &sample.value)
 	}
 
-	semrelease(&metricsSema)
+	metricsUnlock()
 }