Skip to content

Commit

Permalink
[release-branch.go1.20] runtime: set raceignore to zero when starting…
Browse files Browse the repository at this point in the history
… a new goroutine

When reusing a g struct the runtime did not reset
g.raceignore. Initialize raceignore to zero when initially
setting racectx.

A goroutine can end with a non-zero raceignore if it exits
after calling runtime.RaceDisable without a matching
runtime.RaceEnable. If that goroutine's g is later reused
the race detector is in a weird state: the underlying
g.racectx is active, yet g.raceignore is non-zero, and
raceacquire/racerelease which check g.raceignore become
no-ops. This causes the race detector to report races when
there are none.

For #60934
Fixes #60949

Change-Id: Ib8e412f11badbaf69a480f03740da70891f4093f
Reviewed-on: https://go-review.googlesource.com/c/go/+/505055
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
(cherry picked from commit 48dbb62)
Reviewed-on: https://go-review.googlesource.com/c/go/+/505676
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
  • Loading branch information
jellevandenhooff authored and cagedmantis committed Jun 29, 2023
1 parent 08a58dd commit 4db13d7
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 0 deletions.
1 change: 1 addition & 0 deletions src/runtime/proc.go
Original file line number Diff line number Diff line change
Expand Up @@ -4351,6 +4351,7 @@ func newproc1(fn *funcval, callergp *g, callerpc uintptr) *g {
pp.goidcache++
if raceenabled {
newg.racectx = racegostart(callerpc)
newg.raceignore = 0
if newg.labels != nil {
// See note in proflabel.go on labelSync's role in synchronizing
// with the reads in the signal handler.
Expand Down
37 changes: 37 additions & 0 deletions src/runtime/race/testdata/mop_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2092,3 +2092,40 @@ func TestNoRaceTinyAlloc(t *testing.T) {
<-done
}
}

func TestNoRaceIssue60934(t *testing.T) {
// Test that runtime.RaceDisable state doesn't accidentally get applied to
// new goroutines.

// Create several goroutines that end after calling runtime.RaceDisable.
var wg sync.WaitGroup
ready := make(chan struct{})
wg.Add(32)
for i := 0; i < 32; i++ {
go func() {
<-ready // ensure we have multiple goroutines running at the same time
runtime.RaceDisable()
wg.Done()
}()
}
close(ready)
wg.Wait()

// Make sure race detector still works. If the runtime.RaceDisable state
// leaks, the happens-before edges here will be ignored and a race on x will
// be reported.
var x int
ch := make(chan struct{}, 0)
wg.Add(2)
go func() {
x = 1
ch <- struct{}{}
wg.Done()
}()
go func() {
<-ch
_ = x
wg.Done()
}()
wg.Wait()
}

0 comments on commit 4db13d7

Please sign in to comment.