runtime: fail TestGoroutineLeakProfile on data race

Some of the programs in testdata/testgoroutineleakprofile have data
races because they were taken from a corpus that showcases general Go
concurrency bugs, not just leaked goroutines.

This causes some flakiness as tests might fail due to, for example, a
concurrent map access, even outside of race mode.

Let's just call data races a failure and fix them in the examples. As
far as I can tell, there are only two that show up consistently.

Fixes #75732.

Change-Id: I160b3a1cdce4c2de3f2320b68b4083292e02b557
Reviewed-on: https://go-review.googlesource.com/c/go/+/710756
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>

src/runtime/goroutineleakprofile_test.go

@@ -487,7 +487,7 @@ func TestGoroutineLeakProfile(t *testing.T) {
 	testCases = append(testCases, patternTestCases...)
 
 	// Test cases must not panic or cause fatal exceptions.
-	failStates := regexp.MustCompile(`fatal|panic`)
+	failStates := regexp.MustCompile(`fatal|panic|DATA RACE`)
 
 	testApp := func(exepath string, testCases []testCase) {
 
@@ -520,9 +520,9 @@ func TestGoroutineLeakProfile(t *testing.T) {
 						t.Errorf("Test %s produced no output. Is the goroutine leak profile collected?", tcase.name)
 					}
 
-					// Zero tolerance policy for fatal exceptions or panics.
+					// Zero tolerance policy for fatal exceptions, panics, or data races.
 					if failStates.MatchString(runOutput) {
-						t.Errorf("unexpected fatal exception or panic!\noutput:\n%s\n\n", runOutput)
+						t.Errorf("unexpected fatal exception or panic\noutput:\n%s\n\n", runOutput)
 					}
 
 					output += runOutput + "\n\n"
@@ -540,7 +540,7 @@ func TestGoroutineLeakProfile(t *testing.T) {
 				unexpectedLeaks := make([]string, 0, len(foundLeaks))
 
 				// Parse every leak and check if it is expected (maybe as a flaky leak).
-			LEAKS:
+			leaks:
 				for _, leak := range foundLeaks {
 					// Check if the leak is expected.
 					// If it is, check whether it has been encountered before.
@@ -569,7 +569,7 @@ func TestGoroutineLeakProfile(t *testing.T) {
 						for flakyLeak := range tcase.flakyLeaks {
 							if flakyLeak.MatchString(leak) {
 								// The leak is flaky. Carry on to the next line.
-								continue LEAKS
+								continue leaks
 							}
 						}
 

src/runtime/testdata/testgoroutineleakprofile/goker/README.md

@@ -24,18 +24,22 @@ Jingling Xue (jingling@cse.unsw.edu.au):
 
 White paper: https://lujie.ac.cn/files/papers/GoBench.pdf
 
-The examples have been modified in order to run the goroutine leak
+The examples have been modified in order to run the goroutine leak profiler.
-profiler. Buggy snippets are moved from within a unit test to separate
+Buggy snippets are moved from within a unit test to separate applications.
-applications. Each is then independently executed, possibly as multiple
+Each is then independently executed, possibly as multiple copies within the
-copies within the same application in order to exercise more interleavings.
+same application in order to exercise more interleavings. Concurrently, the
-Concurrently, the main program sets up a waiting period (typically 1ms), followed
+main program sets up a waiting period (typically 1ms), followed by a goroutine
-by a goroutine leak profile request. Other modifications may involve injecting calls
+leak profile request. Other modifications may involve injecting calls to
-to `runtime.Gosched()`, to more reliably exercise buggy interleavings, or reductions
+`runtime.Gosched()`, to more reliably exercise buggy interleavings, or reductions
-in waiting periods when calling `time.Sleep`, in order to reduce overall testing time.
+in waiting periods when calling `time.Sleep`, in order to reduce overall testing
+time.
 
-The resulting goroutine leak profile is analyzed to ensure that no unexpected leaks occurred,
+The resulting goroutine leak profile is analyzed to ensure that no unexpecte
-and that the expected leaks did occur. If the leak is flaky, the only purpose of the expected
+leaks occurred, and that the expected leaks did occur. If the leak is flaky, the
-leak list is to protect against unexpected leaks.
+only purpose of the expected leak list is to protect against unexpected leaks.
+
+The examples have also been modified to remove data races, since those create flaky
+test failures, when really all we care about are leaked goroutines.
 
 The entries below document each of the corresponding leaks.
 

src/runtime/testdata/testgoroutineleakprofile/goker/cockroach1055.go

@@ -44,9 +44,9 @@ func (s *Stopper_cockroach1055) SetStopped() {
 func (s *Stopper_cockroach1055) Quiesce() {
 	s.mu.Lock()
 	defer s.mu.Unlock()
-	s.draining = 1
+	atomic.StoreInt32(&s.draining, 1)
 	s.drain.Wait()
-	s.draining = 0
+	atomic.StoreInt32(&s.draining, 0)
 }
 
 func (s *Stopper_cockroach1055) Stop() {

src/runtime/testdata/testgoroutineleakprofile/goker/moby27782.go

@@ -208,6 +208,7 @@ func (container *Container_moby27782) Reset() {
 }
 
 type JSONFileLogger_moby27782 struct {
+	mu      sync.Mutex
 	readers map[*LogWatcher_moby27782]struct{}
 }
 
@@ -218,11 +219,17 @@ func (l *JSONFileLogger_moby27782) ReadLogs() *LogWatcher_moby27782 {
 }
 
 func (l *JSONFileLogger_moby27782) readLogs(logWatcher *LogWatcher_moby27782) {
+	l.mu.Lock()
+	defer l.mu.Unlock()
+
 	l.readers[logWatcher] = struct{}{}
 	followLogs_moby27782(logWatcher)
 }
 
 func (l *JSONFileLogger_moby27782) Close() {
+	l.mu.Lock()
+	defer l.mu.Unlock()
+
 	for r := range l.readers {
 		r.Close()
 		delete(l.readers, r)