runtime/pprof: add race annotations for goroutine profiles

The race annotations for goroutine label maps covered the special type
of read necessary to create CPU profiles. Extend that to include
goroutine profiles. Annotate the copy involved in creating new
goroutines.

Fixes #50292

Change-Id: I10f69314e4f4eba85c506590fe4781f4d6b8ec2d
Reviewed-on: https://go-review.googlesource.com/c/go/+/385660
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Austin Clements <austin@google.com>

diff --git a/src/runtime/mprof.go b/src/runtime/mprof.go
index 569c17f0a7cf4b9ee8f5c3ad3e4f9b5eb52338eb..1edb5d6967ce1885716457df221e945888750ec5 100644 (file)
--- a/src/runtime/mprof.go
+++ b/src/runtime/mprof.go
@@ -818,6 +818,10 @@ func goroutineProfileWithLabels(p []StackRecord, labels []unsafe.Pointer) (n int
                })
        }
 
+       if raceenabled {
+               raceacquire(unsafe.Pointer(&labelSync))
+       }
+
        startTheWorld()
        return n, ok
 }

diff --git a/src/runtime/pprof/pprof_test.go b/src/runtime/pprof/pprof_test.go
index 1742dc0cdcbe1758edbb023c6f579344d8a19a21..1cc69a395e67889efaff78561d2cfd40362778fb 100644 (file)
--- a/src/runtime/pprof/pprof_test.go
+++ b/src/runtime/pprof/pprof_test.go
@@ -1442,7 +1442,7 @@ func TestCPUProfileLabel(t *testing.T) {
 
 func TestLabelRace(t *testing.T) {
        // Test the race detector annotations for synchronization
-       // between settings labels and consuming them from the
+       // between setting labels and consuming them from the
        // profile.
        matches := matchAndAvoidStacks(stackContainsLabeled, []string{"runtime/pprof.cpuHogger;key=value"}, nil)
        testCPUProfile(t, matches, func(dur time.Duration) {
@@ -1464,6 +1464,63 @@ func TestLabelRace(t *testing.T) {
        })
 }
 
+func TestGoroutineProfileLabelRace(t *testing.T) {
+       // Test the race detector annotations for synchronization
+       // between setting labels and consuming them from the
+       // goroutine profile. See issue #50292.
+
+       t.Run("reset", func(t *testing.T) {
+               ctx := context.Background()
+               ctx, cancel := context.WithCancel(ctx)
+               defer cancel()
+
+               go func() {
+                       goroutineProf := Lookup("goroutine")
+                       for ctx.Err() == nil {
+                               var w bytes.Buffer
+                               goroutineProf.WriteTo(&w, 1)
+                               prof := w.String()
+                               if strings.Contains(prof, "loop-i") {
+                                       cancel()
+                               }
+                       }
+               }()
+
+               for i := 0; ctx.Err() == nil; i++ {
+                       Do(ctx, Labels("loop-i", fmt.Sprint(i)), func(ctx context.Context) {
+                       })
+               }
+       })
+
+       t.Run("churn", func(t *testing.T) {
+               ctx := context.Background()
+               ctx, cancel := context.WithCancel(ctx)
+               defer cancel()
+
+               var ready sync.WaitGroup
+               ready.Add(1)
+               var churn func(i int)
+               churn = func(i int) {
+                       SetGoroutineLabels(WithLabels(ctx, Labels("churn-i", fmt.Sprint(i))))
+                       if i == 0 {
+                               ready.Done()
+                       }
+                       if ctx.Err() == nil {
+                               go churn(i + 1)
+                       }
+               }
+               go func() {
+                       churn(0)
+               }()
+               ready.Wait()
+
+               goroutineProf := Lookup("goroutine")
+               for i := 0; i < 10; i++ {
+                       goroutineProf.WriteTo(io.Discard, 1)
+               }
+       })
+}
+
 // TestLabelSystemstack makes sure CPU profiler samples of goroutines running
 // on systemstack include the correct pprof labels. See issue #48577
 func TestLabelSystemstack(t *testing.T) {

diff --git a/src/runtime/proc.go b/src/runtime/proc.go
index f29cc800f79d3a8fea6731c744d3ccc9d74141c0..34b09f2a353c5e90255d746fbf9cc1a68985ea47 100644 (file)
--- a/src/runtime/proc.go
+++ b/src/runtime/proc.go
@@ -4155,6 +4155,11 @@ func newproc1(fn *funcval, callergp *g, callerpc uintptr) *g {
        _p_.goidcache++
        if raceenabled {
                newg.racectx = racegostart(callerpc)
+               if newg.labels != nil {
+                       // See note in proflabel.go on labelSync's role in synchronizing
+                       // with the reads in the signal handler.
+                       racereleasemergeg(newg, unsafe.Pointer(&labelSync))
+               }
        }
        if trace.enabled {
                traceGoCreate(newg, newg.startpc)