-
Notifications
You must be signed in to change notification settings - Fork 18.8k
Description
The waitgroup modernizer rewrites wg.Add(1); go func() { defer wg.Done(); ... }() into wg.Go(func() { ... }), but this changes panic/recover semantics.
With the original pattern, a goroutine can recover() from its own panic and still call wg.Done() (via defer). With wg.Go, panics in the function are not recovered by the function's own deferred recover() in the same way — WaitGroup.Go calls the function and then calls Done itself, but if the function panics, the panic propagates before Done is called, causing wg.Wait() to block forever (or the program to crash).
Before (go fix):
package main
import (
"fmt"
"sync"
"time"
)
func main() {
var wg sync.WaitGroup
done := make(chan struct{})
wg.Add(1)
go func() {
defer func() { recover() }()
panic("x")
wg.Done()
}()
go func() { wg.Wait(); close(done) }()
time.Sleep(100 * time.Millisecond)
select {
case <-done:
fmt.Println("returned")
default:
fmt.Println("blocked")
}
}Output: blocked (the goroutine panics, recover catches it, but wg.Done() is never reached since it's after the panic — so wg.Wait() blocks)
After (GOTOOLCHAIN=go1.26.0 go fix):
package main
import (
"fmt"
"sync"
"time"
)
func main() {
var wg sync.WaitGroup
done := make(chan struct{})
wg.Go(func() {
defer func() { recover() }()
panic("x")
})
go func() { wg.Wait(); close(done) }()
time.Sleep(100 * time.Millisecond)
select {
case <-done:
fmt.Println("returned")
default:
fmt.Println("blocked")
}
}Output: returned (wg.Go defers wg.Done() itself, so even though the function panics and recovers, Done is called by Go's defer, unblocking Wait)
The transformation changes observable behavior when goroutines use panic/recover. The modernizer should not apply the rewrite when the goroutine body contains recover().
CC @adonovan