Skip to content

Commit 4696e77

Browse files
committed
sync: repanic when f() panics for WaitGroup.Go
This is a copy-paste of #76126 (comment) by adonovan. Fixes #76126 Fixes #74702
1 parent ad3ccd9 commit 4696e77

File tree

2 files changed

+50
-1
lines changed

2 files changed

+50
-1
lines changed

src/sync/waitgroup.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,18 @@ func (wg *WaitGroup) Wait() {
236236
func (wg *WaitGroup) Go(f func()) {
237237
wg.Add(1)
238238
go func() {
239-
defer wg.Done()
239+
defer func() {
240+
if x := recover(); x != nil {
241+
// Don't call Done as it may cause Wait to unblock,
242+
// so that the main goroutine races with the runtime.fatal
243+
// resulting from unhandled panic.
244+
panic(x)
245+
}
246+
247+
// f completed normally, or abruptly using goexit.
248+
// Either way, decrement the semaphore.
249+
wg.Done()
250+
}()
240251
f()
241252
}()
242253
}

src/sync/waitgroup_test.go

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,12 @@
55
package sync_test
66

77
import (
8+
"bytes"
9+
"internal/testenv"
10+
"os"
11+
"os/exec"
12+
"strings"
13+
"sync"
814
. "sync"
915
"sync/atomic"
1016
"testing"
@@ -110,6 +116,38 @@ func TestWaitGroupGo(t *testing.T) {
110116
}
111117
}
112118

119+
// This test ensures that an unhandled panic in a Go goroutine terminates
120+
// the process without causing Wait to unblock; previously there was a race.
121+
func TestIssue76126(t *testing.T) {
122+
testenv.MustHaveExec(t)
123+
// Call child in a child process
124+
// and inspect its failure message.
125+
cmd := exec.Command(os.Args[0], "-test.run=^TestIssue76126Child$")
126+
cmd.Env = append(os.Environ(), "SYNC_TEST_CHILD=1")
127+
buf := new(bytes.Buffer)
128+
cmd.Stderr = buf
129+
cmd.Run() // ignore error
130+
131+
got := buf.String()
132+
if strings.Contains(got, "panic: test") {
133+
// ok
134+
} else {
135+
t.Errorf("missing panic: test\n%s", got)
136+
}
137+
}
138+
139+
func TestIssue76126Child(t *testing.T) {
140+
if os.Getenv("SYNC_TEST_CHILD") != "1" {
141+
t.Skip("not child")
142+
}
143+
var wg sync.WaitGroup
144+
wg.Go(func() {
145+
panic("test")
146+
})
147+
wg.Wait() // process should terminate here
148+
panic("Wait returned") // must not be reached
149+
}
150+
113151
func BenchmarkWaitGroupUncontended(b *testing.B) {
114152
type PaddedWaitGroup struct {
115153
WaitGroup

0 commit comments

Comments
 (0)