From e8a53538b473f1a7a92602675eda2d34f3887611 Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Thu, 9 Oct 2025 20:58:34 +0000 Subject: [PATCH] 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 Reviewed-by: Carlos Amedee Auto-Submit: Michael Knyszek Reviewed-by: Cherry Mui --- src/runtime/goroutineleakprofile_test.go | 10 +++---- .../testgoroutineleakprofile/goker/README.md | 30 +++++++++++-------- .../goker/cockroach1055.go | 4 +-- .../goker/moby27782.go | 7 +++++ 4 files changed, 31 insertions(+), 20 deletions(-) diff --git a/src/runtime/goroutineleakprofile_test.go b/src/runtime/goroutineleakprofile_test.go index 6e26bcab13..f5d2dd6372 100644 --- a/src/runtime/goroutineleakprofile_test.go +++ b/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 } } diff --git a/src/runtime/testdata/testgoroutineleakprofile/goker/README.md b/src/runtime/testdata/testgoroutineleakprofile/goker/README.md index 88c50e1e48..e6f8fe23f2 100644 --- a/src/runtime/testdata/testgoroutineleakprofile/goker/README.md +++ b/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 -profiler. Buggy snippets are moved from within a unit test to separate -applications. Each is then independently executed, possibly as multiple -copies within the same application in order to exercise more interleavings. -Concurrently, the main program sets up a waiting period (typically 1ms), followed -by a goroutine leak profile request. Other modifications may involve injecting calls -to `runtime.Gosched()`, to more reliably exercise buggy interleavings, or reductions -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, -and that the expected leaks did occur. If the leak is flaky, the only purpose of the expected -leak list is to protect against unexpected leaks. +The examples have been modified in order to run the goroutine leak profiler. +Buggy snippets are moved from within a unit test to separate applications. +Each is then independently executed, possibly as multiple copies within the +same application in order to exercise more interleavings. Concurrently, the +main program sets up a waiting period (typically 1ms), followed by a goroutine +leak profile request. Other modifications may involve injecting calls to +`runtime.Gosched()`, to more reliably exercise buggy interleavings, or reductions +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 unexpecte +leaks occurred, and that the expected leaks did occur. If the leak is flaky, the +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. @@ -1844,4 +1848,4 @@ c.inbox <- <================> [<-c.inbox] . close(c.closed) . <-c.dispatcherLoopStopped ---------------------G1,G2 leak------------------------------- -``` \ No newline at end of file +``` diff --git a/src/runtime/testdata/testgoroutineleakprofile/goker/cockroach1055.go b/src/runtime/testdata/testgoroutineleakprofile/goker/cockroach1055.go index 687baed25a..87cf157996 100644 --- a/src/runtime/testdata/testgoroutineleakprofile/goker/cockroach1055.go +++ b/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() { diff --git a/src/runtime/testdata/testgoroutineleakprofile/goker/moby27782.go b/src/runtime/testdata/testgoroutineleakprofile/goker/moby27782.go index 7b3398fd38..9b53d9035c 100644 --- a/src/runtime/testdata/testgoroutineleakprofile/goker/moby27782.go +++ b/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) -- 2.52.0