From 1c590661e7d3b477662f76ead56f39567ea8345a Mon Sep 17 00:00:00 2001
From: "Bryan C. Mills" <bcmills@google.com>
Date: Fri, 19 Mar 2021 11:48:22 -0400
Subject: [PATCH] testing: allow parallel-subtest goroutines to exit when the
 subtest is complete

Fixes #45127
Updates #38768

Change-Id: I7f41901d5bcc07741ac9f5f2a24d2b07ef633cb1
Reviewed-on: https://go-review.googlesource.com/c/go/+/303330
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
---
 .../test_finished_subtest_goroutines.txt      | 52 +++++++++++++++++++
 src/testing/testing.go                        | 13 ++---
 2 files changed, 59 insertions(+), 6 deletions(-)
 create mode 100644 src/cmd/go/testdata/script/test_finished_subtest_goroutines.txt

diff --git a/src/cmd/go/testdata/script/test_finished_subtest_goroutines.txt b/src/cmd/go/testdata/script/test_finished_subtest_goroutines.txt
new file mode 100644
index 0000000000..8db821eb77
--- /dev/null
+++ b/src/cmd/go/testdata/script/test_finished_subtest_goroutines.txt
@@ -0,0 +1,52 @@
+# Regression test for https://golang.org/issue/45127:
+# Goroutines for completed parallel subtests should exit immediately,
+# not block until earlier subtests have finished.
+
+[short] skip
+
+! go test .
+stdout 'panic: slow failure'
+! stdout '\[chan send'
+
+-- go.mod --
+module golang.org/issue45127
+
+go 1.16
+-- issue45127_test.go --
+package main
+
+import (
+	"fmt"
+	"runtime"
+	"runtime/debug"
+	"sync"
+	"testing"
+)
+
+func TestTestingGoroutineLeak(t *testing.T) {
+	debug.SetTraceback("all")
+
+	var wg sync.WaitGroup
+	const nFast = 10
+
+	t.Run("slow", func(t *testing.T) {
+		t.Parallel()
+		wg.Wait()
+		for i := 0; i < nFast; i++ {
+			// If the subtest goroutines are going to park on the channel
+			// send, allow them to park now. If they're not going to park,
+			// make sure they have had a chance to run to completion so
+			// that they aren't spuriously parked when we panic.
+			runtime.Gosched()
+		}
+		panic("slow failure")
+	})
+
+	wg.Add(nFast)
+	for i := 0; i < nFast; i++ {
+		t.Run(fmt.Sprintf("leaky%d", i), func(t *testing.T) {
+			t.Parallel()
+			wg.Done()
+		})
+	}
+}
diff --git a/src/testing/testing.go b/src/testing/testing.go
index 383e56a20e..0df6e45ec4 100644
--- a/src/testing/testing.go
+++ b/src/testing/testing.go
@@ -1258,7 +1258,7 @@ func (t *T) Run(name string, f func(t *T)) bool {
 	t = &T{
 		common: common{
 			barrier: make(chan bool),
-			signal:  make(chan bool),
+			signal:  make(chan bool, 1),
 			name:    testName,
 			parent:  &t.common,
 			level:   t.level + 1,
@@ -1539,7 +1539,7 @@ func runTests(matchString func(pat, str string) (bool, error), tests []InternalT
 			ctx.deadline = deadline
 			t := &T{
 				common: common{
-					signal:  make(chan bool),
+					signal:  make(chan bool, 1),
 					barrier: make(chan bool),
 					w:       os.Stdout,
 				},
@@ -1552,11 +1552,12 @@ func runTests(matchString func(pat, str string) (bool, error), tests []InternalT
 				for _, test := range tests {
 					t.Run(test.Name, test.F)
 				}
-				// Run catching the signal rather than the tRunner as a separate
-				// goroutine to avoid adding a goroutine during the sequential
-				// phase as this pollutes the stacktrace output when aborting.
-				go func() { <-t.signal }()
 			})
+			select {
+			case <-t.signal:
+			default:
+				panic("internal error: tRunner exited without sending on t.signal")
+			}
 			ok = ok && !t.Failed()
 			ran = ran || t.ran
 		}