I had the following code as part of a test:
expected := 10
var wg sync.WaitGroup
for i := 0; i < expected; i++ {
go func(wg *sync.WaitGroup) {
wg.Add(1)
defer wg.Done()
// do something
}(&wg)
}
wg.Wait()
To my surprise, I got panic: Fail in goroutine after TestReadWrite has completed
when running "go test". When running with "go test -race", I did not get a panic, but the test failed at a later point. In both cases, despite having a wg.Wait(), a goroutine did not finish executing.
I made the following change, and now the test works as expected:
expected := 10
var wg sync.WaitGroup
wg.Add(expected)
for i := 0; i < expected; i++ {
go func(wg *sync.WaitGroup) {
defer wg.Done()
// do something
}(&wg)
}
wg.Wait()
My doubts are:
wg.Add(1)
inside the goroutine. Why does it behave unexpectedly in this specific case? What seems to be happening here is that some goroutines seem to finish running, and get past the wg.Wait(), before other goroutines even start to run. Is using wg.Add(1) inside the goroutine dangerous / to be avoided? If this is not a problem in general, what exactly is causing the problem here?wg.Add(expected)
the correct way to address this problem?According to the docs -
A WaitGroup waits for a collection of goroutines to finish. The main goroutine calls Add to set the number of goroutines to wait for. Then each of the goroutines runs and calls Done when finished. At the same time, Wait can be used to block until all goroutines have finished.
So Add()
must be called by a goroutine which is initiating other goroutines which in your case is the main
goroutine.
In the first code snippet, you are calling Add()
inside other goroutines instead of the main goroutine which is causing problem -
expected := 10
var wg sync.WaitGroup
for i := 0; i < expected; i++ {
go func(wg *sync.WaitGroup) {
wg.Add(1) // Do not call Add() here
defer wg.Done()
// do something
}(&wg)
}
wg.Wait()
The second snippet is working because you are calling Add()
in the main
goroutine -
expected := 10
var wg sync.WaitGroup
wg.Add(expected) // Okay
for i := 0; i < expected; i++ {
go func(wg *sync.WaitGroup) {
defer wg.Done()
// do something
}(&wg)
}
wg.Wait()
Is adding wg.Add(expected) the correct way to address this problem?
You can also call wg.Add(1)
in the for loop -
expected := 10
var wg sync.WaitGroup
for i := 0; i < expected; i++ {
wg.Add(1) // Okay
go func(wg *sync.WaitGroup) {
defer wg.Done()
// do something
}(&wg)
}
wg.Wait()