From 468d3f4451d651ca70081e723b74af452b570517 Mon Sep 17 00:00:00 2001 From: Jes Cok Date: Wed, 5 Nov 2025 00:55:49 +0800 Subject: [PATCH 1/3] sync: repanic when f() panics for WaitGroup.Go This is a copy-paste of https://github.com/golang/go/issues/76126#issuecomment-3473417226 by adonovan. Fixes #76126 Fixes #74702 --- src/sync/waitgroup.go | 13 ++++++++++++- src/sync/waitgroup_test.go | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/src/sync/waitgroup.go b/src/sync/waitgroup.go index 195f839da417bd..dcc5636c4bd80c 100644 --- a/src/sync/waitgroup.go +++ b/src/sync/waitgroup.go @@ -236,7 +236,18 @@ func (wg *WaitGroup) Wait() { func (wg *WaitGroup) Go(f func()) { wg.Add(1) go func() { - defer wg.Done() + defer func() { + if x := recover(); x != nil { + // Don't call Done as it may cause Wait to unblock, + // so that the main goroutine races with the runtime.fatal + // resulting from unhandled panic. + panic(x) + } + + // f completed normally, or abruptly using goexit. + // Either way, decrement the semaphore. + wg.Done() + }() f() }() } diff --git a/src/sync/waitgroup_test.go b/src/sync/waitgroup_test.go index 8a948f8972c8a7..929e0e9055b984 100644 --- a/src/sync/waitgroup_test.go +++ b/src/sync/waitgroup_test.go @@ -5,6 +5,12 @@ package sync_test import ( + "bytes" + "internal/testenv" + "os" + "os/exec" + "strings" + "sync" . "sync" "sync/atomic" "testing" @@ -110,6 +116,38 @@ func TestWaitGroupGo(t *testing.T) { } } +// This test ensures that an unhandled panic in a Go goroutine terminates +// the process without causing Wait to unblock; previously there was a race. +func TestIssue76126(t *testing.T) { + testenv.MustHaveExec(t) + // Call child in a child process + // and inspect its failure message. + cmd := exec.Command(os.Args[0], "-test.run=^TestIssue76126Child$") + cmd.Env = append(os.Environ(), "SYNC_TEST_CHILD=1") + buf := new(bytes.Buffer) + cmd.Stderr = buf + cmd.Run() // ignore error + + got := buf.String() + if strings.Contains(got, "panic: test") { + // ok + } else { + t.Errorf("missing panic: test\n%s", got) + } +} + +func TestIssue76126Child(t *testing.T) { + if os.Getenv("SYNC_TEST_CHILD") != "1" { + t.Skip("not child") + } + var wg sync.WaitGroup + wg.Go(func() { + panic("test") + }) + wg.Wait() // process should terminate here + panic("Wait returned") // must not be reached +} + func BenchmarkWaitGroupUncontended(b *testing.B) { type PaddedWaitGroup struct { WaitGroup From b8cd2220c271b26acbd9b1bec371f9a042e134c5 Mon Sep 17 00:00:00 2001 From: Jes Cok Date: Fri, 7 Nov 2025 02:43:28 +0800 Subject: [PATCH 2/3] 2 --- src/sync/waitgroup_test.go | 35 +++++++++++++++-------------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/src/sync/waitgroup_test.go b/src/sync/waitgroup_test.go index 929e0e9055b984..2c6ce73c21a2e5 100644 --- a/src/sync/waitgroup_test.go +++ b/src/sync/waitgroup_test.go @@ -10,7 +10,6 @@ import ( "os" "os/exec" "strings" - "sync" . "sync" "sync/atomic" "testing" @@ -120,27 +119,23 @@ func TestWaitGroupGo(t *testing.T) { // the process without causing Wait to unblock; previously there was a race. func TestIssue76126(t *testing.T) { testenv.MustHaveExec(t) - // Call child in a child process - // and inspect its failure message. - cmd := exec.Command(os.Args[0], "-test.run=^TestIssue76126Child$") - cmd.Env = append(os.Environ(), "SYNC_TEST_CHILD=1") - buf := new(bytes.Buffer) - cmd.Stderr = buf - cmd.Run() // ignore error - - got := buf.String() - if strings.Contains(got, "panic: test") { - // ok - } else { - t.Errorf("missing panic: test\n%s", got) - } -} - -func TestIssue76126Child(t *testing.T) { if os.Getenv("SYNC_TEST_CHILD") != "1" { - t.Skip("not child") + // Call child in a child process + // and inspect its failure message. + cmd := exec.Command(os.Args[0], "-test.run=^TestIssue76126$") + cmd.Env = append(os.Environ(), "SYNC_TEST_CHILD=1") + buf := new(bytes.Buffer) + cmd.Stderr = buf + cmd.Run() // ignore error + got := buf.String() + if strings.Contains(got, "panic: test") { + // ok + } else { + t.Errorf("missing panic: test\n%s", got) + } + return } - var wg sync.WaitGroup + var wg WaitGroup wg.Go(func() { panic("test") }) From 5d81a7e5fb124ba73e3f547ce064f8091a85447d Mon Sep 17 00:00:00 2001 From: Jes Cok Date: Sat, 8 Nov 2025 05:55:36 +0800 Subject: [PATCH 3/3] 3 --- src/sync/waitgroup.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/sync/waitgroup.go b/src/sync/waitgroup.go index dcc5636c4bd80c..29117ab9a9d3ed 100644 --- a/src/sync/waitgroup.go +++ b/src/sync/waitgroup.go @@ -238,9 +238,16 @@ func (wg *WaitGroup) Go(f func()) { go func() { defer func() { if x := recover(); x != nil { - // Don't call Done as it may cause Wait to unblock, - // so that the main goroutine races with the runtime.fatal - // resulting from unhandled panic. + // f panicked, which will be fatal because + // this is a new goroutine. + // + // Calling Done will unblock Wait in the main goroutine, + // allowing it to race with the fatal panic and + // possibly even exit the process (os.Exit(0)) + // before the panic completes. + // + // This is almost certainly undesirable, + // so instead avoid calling Done and simply panic. panic(x) }