Skip to content

Commit 209942f

Browse files
rhyshmknyszek
authored andcommitted
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 <[email protected]> Reviewed-by: Austin Clements <[email protected]>
1 parent 7c404d5 commit 209942f

File tree

3 files changed

+67
-1
lines changed

3 files changed

+67
-1
lines changed

src/runtime/mprof.go

+4
Original file line numberDiff line numberDiff line change
@@ -818,6 +818,10 @@ func goroutineProfileWithLabels(p []StackRecord, labels []unsafe.Pointer) (n int
818818
})
819819
}
820820

821+
if raceenabled {
822+
raceacquire(unsafe.Pointer(&labelSync))
823+
}
824+
821825
startTheWorld()
822826
return n, ok
823827
}

src/runtime/pprof/pprof_test.go

+58-1
Original file line numberDiff line numberDiff line change
@@ -1442,7 +1442,7 @@ func TestCPUProfileLabel(t *testing.T) {
14421442

14431443
func TestLabelRace(t *testing.T) {
14441444
// Test the race detector annotations for synchronization
1445-
// between settings labels and consuming them from the
1445+
// between setting labels and consuming them from the
14461446
// profile.
14471447
matches := matchAndAvoidStacks(stackContainsLabeled, []string{"runtime/pprof.cpuHogger;key=value"}, nil)
14481448
testCPUProfile(t, matches, func(dur time.Duration) {
@@ -1464,6 +1464,63 @@ func TestLabelRace(t *testing.T) {
14641464
})
14651465
}
14661466

1467+
func TestGoroutineProfileLabelRace(t *testing.T) {
1468+
// Test the race detector annotations for synchronization
1469+
// between setting labels and consuming them from the
1470+
// goroutine profile. See issue #50292.
1471+
1472+
t.Run("reset", func(t *testing.T) {
1473+
ctx := context.Background()
1474+
ctx, cancel := context.WithCancel(ctx)
1475+
defer cancel()
1476+
1477+
go func() {
1478+
goroutineProf := Lookup("goroutine")
1479+
for ctx.Err() == nil {
1480+
var w bytes.Buffer
1481+
goroutineProf.WriteTo(&w, 1)
1482+
prof := w.String()
1483+
if strings.Contains(prof, "loop-i") {
1484+
cancel()
1485+
}
1486+
}
1487+
}()
1488+
1489+
for i := 0; ctx.Err() == nil; i++ {
1490+
Do(ctx, Labels("loop-i", fmt.Sprint(i)), func(ctx context.Context) {
1491+
})
1492+
}
1493+
})
1494+
1495+
t.Run("churn", func(t *testing.T) {
1496+
ctx := context.Background()
1497+
ctx, cancel := context.WithCancel(ctx)
1498+
defer cancel()
1499+
1500+
var ready sync.WaitGroup
1501+
ready.Add(1)
1502+
var churn func(i int)
1503+
churn = func(i int) {
1504+
SetGoroutineLabels(WithLabels(ctx, Labels("churn-i", fmt.Sprint(i))))
1505+
if i == 0 {
1506+
ready.Done()
1507+
}
1508+
if ctx.Err() == nil {
1509+
go churn(i + 1)
1510+
}
1511+
}
1512+
go func() {
1513+
churn(0)
1514+
}()
1515+
ready.Wait()
1516+
1517+
goroutineProf := Lookup("goroutine")
1518+
for i := 0; i < 10; i++ {
1519+
goroutineProf.WriteTo(io.Discard, 1)
1520+
}
1521+
})
1522+
}
1523+
14671524
// TestLabelSystemstack makes sure CPU profiler samples of goroutines running
14681525
// on systemstack include the correct pprof labels. See issue #48577
14691526
func TestLabelSystemstack(t *testing.T) {

src/runtime/proc.go

+5
Original file line numberDiff line numberDiff line change
@@ -4155,6 +4155,11 @@ func newproc1(fn *funcval, callergp *g, callerpc uintptr) *g {
41554155
_p_.goidcache++
41564156
if raceenabled {
41574157
newg.racectx = racegostart(callerpc)
4158+
if newg.labels != nil {
4159+
// See note in proflabel.go on labelSync's role in synchronizing
4160+
// with the reads in the signal handler.
4161+
racereleasemergeg(newg, unsafe.Pointer(&labelSync))
4162+
}
41584163
}
41594164
if trace.enabled {
41604165
traceGoCreate(newg, newg.startpc)

0 commit comments

Comments
 (0)