Example for sync.WaitGroup correct?

Question

Is this example usage of sync.WaitGroup correct? It gives the expected result, but I am unsure about the wg.Add(4) and the position of wg.Done(). Does it make sense to add the four goroutines at once with wg.Add()?

http://play.golang.org/p/ecvYHiie0P

package main import ( "fmt" "sync" "time" ) func dosomething(millisecs time.Duration, wg *sync.WaitGroup) { duration := millisecs * time.Millisecond time.Sleep(duration) fmt.Println("Function in background, duration:", duration) wg.Done() } func main() { var wg sync.WaitGroup wg.Add(4) go dosomething(200, &wg) go dosomething(400, &wg) go dosomething(150, &wg) go dosomething(600, &wg) wg.Wait() fmt.Println("Done") }

Result (as expected):

Function in background, duration: 150ms Function in background, duration: 200ms Function in background, duration: 400ms Function in background, duration: 600ms Done

I realize this is old, but for future people, I would recommend an initial defer wg.Done() call at the start of the function. — Brian
– Brian, Commented Nov 9, 2017 at 13:33

Stephen Weinberg · Accepted Answer · 2018-06-09 14:18:06Z

Yes, this example is correct. It is important that the wg.Add() happens before the go statement to prevent race conditions. The following would also be correct:

func main() { var wg sync.WaitGroup wg.Add(1) go dosomething(200, &wg) wg.Add(1) go dosomething(400, &wg) wg.Add(1) go dosomething(150, &wg) wg.Add(1) go dosomething(600, &wg) wg.Wait() fmt.Println("Done") }

However, it is rather pointless to call wg.Add over and over again when you already know how many times it will be called.

Waitgroups panic if the counter falls below zero. The counter starts at zero, each Done() is a -1 and each Add() depends on the parameter. So, to ensure that the counter never drops below and avoid panics, you need the Add() to be guaranteed to come before the Done().

In Go, such guarantees are given by the memory model.

The memory model states that all statements in a single goroutine appear to be executed in the same order as they are written. It is possible that they won't actually be in that order, but the outcome will be as if it was. It is also guaranteed that a goroutine doesn't run until after the go statement that calls it. Since the Add() occurs before the go statement and the go statement occurs before the Done(), we know the Add() occurs before the Done().

If you were to have the go statement come before the Add(), the program may operate correctly. However, it would be a race condition because it would not be guaranteed.

I have a question over this one: wouldn't it be better to defer wg.Done() so that we are sure that it gets called irrespective of the route the goroutine takes? Thanks.
if you purely wanted to make sure the function didn't return before all the go routines were finished then yes defer would be preferred. just usually the entire point of a wait group is to wait till all the work is done to then do something with the results you were waiting for.
If you don't use defer and one of your goroutines fails to call wg.Done() ... won't your Wait simply block forever? This sounds like it could easily introduce a hard-to-find bug into your code...

Community · Accepted Answer · 2017-05-23 12:03:04Z

I would recommend embeding the wg.Add() call into the doSomething() function itself, so that if you adjust the number of times it's called, you don't have to separately adjust the add parameter manually which could lead to an error if you update one but forget to update the other (in this trivial example that is unlikely, but still, I personally believe it to be better practice for code re-use).

As Stephen Weinberg points out in his answer to this question, you do have to increment the waitgroup prior to spawning the gofunc, but you can accomplish this easily by wrapping the gofunc spawn inside the doSomething() function itself, like this:

func dosomething(millisecs time.Duration, wg *sync.WaitGroup) { wg.Add(1) go func() { duration := millisecs * time.Millisecond time.Sleep(duration) fmt.Println("Function in background, duration:", duration) wg.Done() }() }

Then you can call it without the go invocation, e.g.:

func main() { var wg sync.WaitGroup dosomething(200, &wg) dosomething(400, &wg) dosomething(150, &wg) dosomething(600, &wg) wg.Wait() fmt.Println("Done") }

As a playground: http://play.golang.org/p/WZcprjpHa_

Bnaya · Accepted Answer · 2018-07-17 17:39:53Z

small improvement based on Mroth answer
using defer for Done is safer

 func dosomething(millisecs time.Duration, wg *sync.WaitGroup) { wg.Add(1) go func() { defer wg.Done() duration := millisecs * time.Millisecond time.Sleep(duration) fmt.Println("Function in background, duration:", duration) }() } func main() { var wg sync.WaitGroup dosomething(200, &wg) dosomething(400, &wg) dosomething(150, &wg) dosomething(600, &wg) wg.Wait() fmt.Println("Done") }

Collectives™ on Stack Overflow

Example for sync.WaitGroup correct?

3 Answers 3

3 Comments

Comments

Comments

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

3 Comments

Comments

Comments

Linked

Related