Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Why does this WaitGroup sometimes not wait for all goroutines?

Tags:

concurrency

go

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.

like image 403
Sridhar Avatar asked Dec 14 '22 04:12

Sridhar


2 Answers

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.

like image 148
abhink Avatar answered Dec 16 '22 18:12

abhink


If you run your code with race detector

go test -race .

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

like image 30
Grzegorz Żur Avatar answered Dec 16 '22 17:12

Grzegorz Żur