CODEWITHSUNDEEP
goconcurrency
honzajde
honzajde

Reputation: 2398

Does WaitGroup.Wait() imply memory barrier in this case?

var condition bool
var wg sync.WaitGroup
for _, item := range items {
    wg.Add(1)
    go func(item) {
        if meetsCondition(item) {
            condition = true
        }
        wg.Done()
    }(item)
}
wg.Wait()
// is it safe to check condition here?

There is a discussion of this question at the old go forum here: https://groups.google.com/forum/#!topic/golang-nuts/5oHzhzXCcmM The answer there was yes, it is safe. Then the discussion digress to use of atomic, etc., which is not what I want to ask about.

There is not even one mention of WaitGroup in the spec and it's documentation is saying WaitGroup.Wait: "Wait blocks until the WaitGroup counter is zero." which does not set any happens-before relationship (or does it?).

Does it mean that the first answer "It is safe to check condition after wg.Wait returns" is unofficial? If it's official what are the reasons for it I am missing? Thanks alot if you answer.

Update: This is update after @peterSO's @ravi's answers. Thanks.

Well, obviously there can be a race condition if you choose number of items > 1. Hint: condition can be set only to true. Still, I have the same question.

And probably, I should have stated that:

  1. underlying architectures can be only x86, x64 or ARM
  2. number of items can be 1

Update 2 I created followup question for the case when number of items == 1 here: Can you make this 'incorrectly synchronized' test fail?

Upvotes: 6

Views: 1001

Answers (2)

honzajde
honzajde

Reputation: 2398

Yes. There is happens-before relationship between wg.Wait() and wg.Done(). This simple fact is for whatever reason not mentioned in doc or go MM #7948. Thanks Ian Lance Taylor for clarifying it golang-nuts/5oHzhzXCcmM. About the race condition you can read more in the same thread.

A bit disappointing that in a language that calls itself concurrent one has to rely on a 'good word' to do the basic stuff (right).

Upvotes: 3

Ravi R
Ravi R

Reputation: 1782

Well, it is absolutely safe to check condition after wg.Wait(), but you should protect condition with a mutex to avoid it being "written to" simultaneously by multiple go routines. That's why @peterSO is hitting a race condition b'cos at line 20 in his code which is setting condition = true multiple go routines are trying to set condition at the same time. Here's a sample https://play.golang.org/p/o3v6s_2qsY with 20k go rountines.

As a best practice I recommend, one adds defer wg.Done() right in the beginning of the go routine function, so that even if there are return statements in between, wg.Done() still gets called.

Upvotes: 2

Related Questions