mirror of
https://github.com/golang/go.git
synced 2025-12-08 06:10:04 +00:00
sync: don't call Done when f() panics in WaitGroup.Go
This change is based on
https://github.com/golang/go/issues/76126#issuecomment-3473417226
by adonovan.
Fixes #76126
Fixes #74702
Change-Id: Ie404d8204be8917fa8a7b414bb6d319238267c83
GitHub-Last-Rev: b1beddcd72
GitHub-Pull-Request: golang/go#76172
Reviewed-on: https://go-review.googlesource.com/c/go/+/717760
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Mateusz Poliwczak <mpoliwczak34@gmail.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
This commit is contained in:
parent
e8ed85d6c2
commit
95a0e5adc1
2 changed files with 50 additions and 1 deletions
|
|
@ -236,7 +236,25 @@ 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 {
|
||||
// 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)
|
||||
}
|
||||
|
||||
// f completed normally, or abruptly using goexit.
|
||||
// Either way, decrement the semaphore.
|
||||
wg.Done()
|
||||
}()
|
||||
f()
|
||||
}()
|
||||
}
|
||||
|
|
|
|||
|
|
@ -5,6 +5,11 @@
|
|||
package sync_test
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"internal/testenv"
|
||||
"os"
|
||||
"os/exec"
|
||||
"strings"
|
||||
. "sync"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
|
|
@ -110,6 +115,32 @@ 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)
|
||||
if os.Getenv("SYNC_TEST_CHILD") != "1" {
|
||||
// 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") {
|
||||
t.Errorf("missing panic: test\n%s", got)
|
||||
}
|
||||
return
|
||||
}
|
||||
var wg 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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue