testing: a panic in a benchmark can be lost; the run prints PASS and exits 0 #82299

Go version

go version go1.27.0 linux/amd64

Output of go env in your module/workspace:

AR='ar'
CC='gcc'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='g++'
GCCGO='gccgo'
GO111MODULE=''
GOAMD64='v1'
GOARCH='amd64'
GOAUTH='netrc'
GOBIN='/home/ubuntu/go/bin'
GOCACHE='/home/ubuntu/.cache/go-build'
GOCACHEPROG=''
GODEBUG=''
GOENV='/home/ubuntu/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build2014841333=/tmp/go-build -gno-record-gcc-switches'
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMOD='/tmp/repro/go.mod'
GOMODCACHE='/home/ubuntu/go/pkg/mod'
GONOPROXY='github.com/mvrahden/*'
GONOSUMDB='github.com/mvrahden/*'
GOOS='linux'
GOPACKAGESDRIVER=''
GOPATH='/home/ubuntu/go'
GOPRIVATE='github.com/mvrahden/*'
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTELEMETRY='local'
GOTELEMETRYDIR='/home/ubuntu/.config/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.27.0'
GOWORK=''
PKG_CONFIG='pkg-config'

What did you do?

A benchmark that panics with a value whose Error method is slow. The runtime calls that method to print the panic before it ends the process, which widens the window this bug needs, so the result is deterministic:

// repro_test.go, in a module with `go 1.26`
package repro

import (
	"testing"
	"time"
)

type slowErr struct{}

func (slowErr) Error() string {
	time.Sleep(time.Second)
	return "boom"
}

func BenchmarkPanic(b *testing.B) {
	panic(slowErr{})
}

go test -run '^$' -bench . -benchtime=1x -count=1 .

The same happens without the slow Error method, only rarely. A benchmark whose body is just panic("boom") loses the panic about 1 run in 200 on a busy machine. This loop runs its compiled test binary 2000 times, eight at a time, and prints how many runs exited 0 (it should print 0):

go test -c -o repro.test .
for j in $(seq 1 8); do
  ( for i in $(seq 1 250); do
      ./repro.test -test.run '^$' -test.bench . -test.benchtime=1x >/dev/null 2>&1 && echo "exit 0"
    done ) &
done | wc -l
wait

What did you see happen?

With the slow Error method, every run reports the benchmark as passing. The panic is never printed:

goos: linux
goarch: amd64
pkg: repro
cpu: AMD Ryzen 9 7950X3D 16-Core Processor
BenchmarkPanic-6   	       1
PASS
ok  	repro	0.004s

The test binary run directly exits 0 in 10 of 10 runs, on go1.27.0 and go1.26.9 alike.

With panic("boom"), most runs panic and exit 2 as expected, but some print the same PASS and exit 0:

The panic is lost wherever it happens on the benchmark's goroutine. I measured 1600 runs each on go1.27.0 for:

• a panic in a b.Cleanup function of a benchmark with a sub-benchmark: 8 exited 0;
• a panic in the benchmark body after a sub-benchmark ran: 8 exited 0;
• a panic in a sub-benchmark's body: 5 exited 0.

We first saw it as an intermittent CI failure on windows-latest (GitHub Actions): a benchmark whose cleanup panics printed PASS and exited 0.

What did you expect to see?

Every run should end with the panic and its traceback, a non-zero exit status, and FAIL from the go command, as a panicking test does. A benchmark that panics must never be reported as passing.

What seems to happen: (*B).run1 (and likewise (*B).launch) runs the benchmark on a new goroutine whose deferred function sends on b.signal. The comment there says this is so that a return through FailNow's runtime.Goexit also signals. But deferred functions also run while a panic unwinds, so a panic signals the benchmark done too. The goroutine waiting on <-b.signal then carries on as if the benchmark had finished: it prints the result, the run prints PASS, and main calls os.Exit(0). If it gets there before the panicking goroutine finishes unwinding and the runtime ends the process, the panic is never printed. tRunner does not have this problem for tests: its deferred function tells a panic from a Goexit and re-raises the panic without signalling the parent, so nothing goes on as if the test had finished.

Proposed fix

Signal only when the benchmark goroutine returned, or stopped with FailNow or SkipNow, and re-raise anything else, as tRunner does for tests. Both run1 and launch defer one helper:

diff --git a/src/testing/benchmark.go b/src/testing/benchmark.go
index ad25639180..1083467993 100644
--- a/src/testing/benchmark.go
+++ b/src/testing/benchmark.go
@@ -236,13 +236,11 @@ func (b *B) run1() bool {
 		}
 	}
 	go func() {
-		// Signal that we're done whether we return normally
-		// or by FailNow's runtime.Goexit.
-		defer func() {
-			b.signal <- true
-		}()
+		returned := false
+		defer b.signalDone(&returned)
 
 		b.runN(1)
+		returned = true
 	}()
 	<-b.signal
 	if b.failed {
@@ -329,11 +327,8 @@ func predictN(goalns int64, prevIters int64, prevns int64, last int64) int {
 // launch is run by the doBench function as a separate goroutine.
 // run1 must have been called on b.
 func (b *B) launch() {
-	// Signal that we're done whether we return normally
-	// or by FailNow's runtime.Goexit.
-	defer func() {
-		b.signal <- true
-	}()
+	returned := false
+	defer b.signalDone(&returned)
 
 	// b.Loop does its own ramp-up logic so we just need to run it once.
 	// If b.loop.n is non zero, it means b.Loop has already run.
@@ -359,6 +354,40 @@ func (b *B) launch() {
 		}
 	}
 	b.result = BenchmarkResult{b.N, b.duration, b.bytes, b.netAllocs, b.netBytes, b.extra}
+	returned = true
+}
+
+// signalDone must be deferred directly by the goroutine that runs b, with
+// returned reporting whether that goroutine's function returned normally.
+// It signals that b is done whether the benchmark returned or stopped with
+// FailNow or SkipNow. A panic is re-raised instead of signaled: the waiting
+// goroutine would go on as if b had finished and could exit the process
+// before the panic is reported. Like tRunner, it also treats a stop that is
+// neither a return, a FailNow or SkipNow, nor a panic as a panic: a
+// panic(nil) under GODEBUG=panicnil=1, or a direct call to runtime.Goexit.
+func (b *B) signalDone(returned *bool) {
+	err := recover()
+	if err == nil && !*returned && !b.stopped() {
+		err = errNilPanicOrGoexit
+	}
+	if err != nil {
+		panic(err)
+	}
+	b.signal <- true
+}
+
+// stopped reports whether b or one of its parents stopped with FailNow or
+// SkipNow.
+func (b *B) stopped() bool {
+	for c := &b.common; c != nil; c = c.parent {
+		c.mu.RLock()
+		finished := c.finished
+		c.mu.RUnlock()
+		if finished {
+			return true
+		}
+	}
+	return false
 }
 
 // Elapsed returns the measured elapsed time of the benchmark.

• A panic is re-raised without sending on b.signal. The waiting goroutine stays blocked until the panic ends the process, and the traceback still starts at the panicking line, below a [recovered] marker, as for a test.
• FailNow and SkipNow set finished before runtime.Goexit, so they signal as before. A sub-benchmark that stopped through a parent's FailNow also counts as stopped, since the helper checks the parents too.
• Any other stop fails as it does in a test, with test executed panic(nil) or runtime.Goexit. That covers a direct runtime.Goexit and a panic(nil) under GODEBUG=panicnil=1, which recover would otherwise swallow. This changes behavior: today a benchmark that calls runtime.Goexit passes silently.

The change comes with a cmd/go script test, test_benchmark_panic.txt. It covers a panic in the first iteration (run1), in a later one (launch), in a sub-benchmark, and a direct runtime.Goexit. Each panic value has a slow Error method, so without the fix every case reports ok, deterministically.

Tested on master (dfc6f7d):

• go test testing passes, and so do the cmd/go test_benchmark* script tests.
• The new script test fails without the fix ("unexpected success") and passes with it.
• Each of its cases reports ok on an unpatched go1.27.0.
• b.Fatal still fails with exit 1; b.Skip and ordinary benchmarks still pass.
• all.bash passes.

I have the change ready and would like to send it as a CL once the approach is agreed.

Possibly related: #60129 (parent B.Cleanup functions do not run if a sub-benchmark panics), which is about the cleanup order around a sub-benchmark panic, not about a lost verdict.