testing: fix Cleanup race with Logf and Errorf

Fixes #40908

Change-Id: I25561a3f18e730a50e6fbf85aa7bd85bf1b73b6e
Reviewed-on: https://go-review.googlesource.com/c/go/+/250078
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/cmd/go/testdata/script/testing_issue40908.txt b/src/cmd/go/testdata/script/testing_issue40908.txt
new file mode 100644 (file)
index 0000000..4939de0
--- /dev/null
+++ b/src/cmd/go/testdata/script/testing_issue40908.txt
@@ -0,0 +1,21 @@
+[short] skip
+[!race] skip
+
+go test -race testrace
+
+-- testrace/race_test.go --
+package testrace
+
+import "testing"
+
+func TestRace(t *testing.T) {
+       helperDone := make(chan struct{})
+       go func() {
+               t.Logf("Something happened before cleanup.")
+               close(helperDone)
+       }()
+
+       t.Cleanup(func() {
+               <-helperDone
+       })
+}

diff --git a/src/testing/testing.go b/src/testing/testing.go
index 6fc8c4fa9f843b2630c134c6cb550a34933fb90d..bf83df8863244346b13be6152eba7d75f97fe764 100644 (file)
--- a/src/testing/testing.go
+++ b/src/testing/testing.go
@@ -860,11 +860,15 @@ func (c *common) Cleanup(f func()) {
        c.cleanup = func() {
                if oldCleanup != nil {
                        defer func() {
+                               c.mu.Lock()
                                c.cleanupPc = oldCleanupPc
+                               c.mu.Unlock()
                                oldCleanup()
                        }()
                }
+               c.mu.Lock()
                c.cleanupName = callerName(0)
+               c.mu.Unlock()
                f()
        }
        var pc [maxStackLen]uintptr