Search code examples
gosynchronizationrace-conditiongoroutinewaitgroup

sync.WaitGroup initialization before goroutine start


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:

  1. A lot of the code I have seen so far does 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?
  2. Is adding wg.Add(expected) the correct way to address this problem?

Solution

  • 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()