CODEWITHSUNDEEP
goconcurrency
Sridhar
Sridhar

Reputation: 2532

Why does this WaitGroup sometimes not wait for all goroutines?

The code below outputs 2 sometimes. Why isn't the wait group waiting for all the goroutines to complete ?

type Scratch struct {
    //sync.RWMutex
    Itch []int
}

func (s *Scratch) GoScratch(done chan bool, j int) error {

    var ws sync.WaitGroup

    if len(s.Itch) == 0 {
            s.Rash = make([]int, 0)
    }
    for i := 0; i < j; i++ {
            ws.Add(1)
            go func (i int) {
                    defer ws.Done()

                   s.Rash = append(s.Rash, i) 
            }(i)
    }
    ws.Wait()
    done<- true
    return nil
}

func main() {
    done := make(chan bool, 3)
    s := &Scratch{}
    err := s.GoScratch(done, 3)
    if err != nil {
            log.Println("Error:%v",err)
    }
    <-done
    log.Println("Length: ", len(s.Rash)) 
}`

Strangely i can't get it to output 2 with a main function but when I use a test case it outputs 2 sometimes.

Upvotes: 6

Views: 3132

Answers (2)

Grzegorz Żur
Grzegorz Żur

Reputation: 49251

If you run your code with race detector 

go test -race .

you will find out race condition on slice s.Rash.

Upvotes: 6

abhink
abhink

Reputation: 9136

There is a race condition in your code. It is right here:

go func (i int) {
    defer ws.Done()
    // race condition on s.Rash access
    s.Rash = append(s.Rash, i) 
}(i)

Since all the goroutines access s.Rash concurrently, this may cause the slice updates to be overwritten. Try running the same code with sync.Mutex locking to prevent this:

// create a global mutex
var mutex = &sync.Mutex{}

// use mutex to prevent race condition
go func (i int) {
    defer ws.Done()
    defer mutex.Unlock() // ensure that mutex unlocks

    // Lock the resource before accessing it
    mutex.Lock()
    s.Rash = append(s.Rash, i) 
}(i)

You can read more about this here and here.

Upvotes: 11

Related Questions