sync: fix spurious wakeup from WaitGroup.Wait

There is a race condition that causes spurious wakeup from Wait
in the following case:

 G1: decrement wg.counter, observe the counter is now 0
     (should unblock goroutines queued *at this moment*)
 G2: increment wg.counter
 G2: call Wait() to add itself to the wait queue
 G1: acquire wg.m, unblock all waiting goroutines

In the last step G2 is spuriously woken up by G1.
Fixes #7734.

LGTM=rsc, dvyukov
R=dvyukov, 0xjnml, rsc
CC=golang-codereviews
https://golang.org/cl/85580043

diff --git a/src/pkg/sync/waitgroup.go b/src/pkg/sync/waitgroup.go
index 22681115cb670064b0d76dc5ce6adf76c565076b..4c64dca393fe0eec79f4631646c576d64fe98620 100644 (file)
--- a/src/pkg/sync/waitgroup.go
+++ b/src/pkg/sync/waitgroup.go
@@ -67,11 +67,13 @@ func (wg *WaitGroup) Add(delta int) {
                return
        }
        wg.m.Lock()
-       for i := int32(0); i < wg.waiters; i++ {
-               runtime_Semrelease(wg.sema)
+       if atomic.LoadInt32(&wg.counter) == 0 {
+               for i := int32(0); i < wg.waiters; i++ {
+                       runtime_Semrelease(wg.sema)
+               }
+               wg.waiters = 0
+               wg.sema = nil
        }
-       wg.waiters = 0
-       wg.sema = nil
        wg.m.Unlock()
 }
 

diff --git a/src/pkg/sync/waitgroup_test.go b/src/pkg/sync/waitgroup_test.go
index 0cbd51056a7e17f89850bbf9794e8968347e7e72..4c0a043c01ee7d6ce058c59032e4ade62075e6b0 100644 (file)
--- a/src/pkg/sync/waitgroup_test.go
+++ b/src/pkg/sync/waitgroup_test.go
@@ -6,6 +6,7 @@ package sync_test
 
 import (
        . "sync"
+       "sync/atomic"
        "testing"
 )
 
@@ -59,6 +60,31 @@ func TestWaitGroupMisuse(t *testing.T) {
        t.Fatal("Should panic")
 }
 
+func TestWaitGroupRace(t *testing.T) {
+       // Run this test for about 1ms.
+       for i := 0; i < 1000; i++ {
+               wg := &WaitGroup{}
+               n := new(int32)
+               // spawn goroutine 1
+               wg.Add(1)
+               go func() {
+                       atomic.AddInt32(n, 1)
+                       wg.Done()
+               }()
+               // spawn goroutine 2
+               wg.Add(1)
+               go func() {
+                       atomic.AddInt32(n, 1)
+                       wg.Done()
+               }()
+               // Wait for goroutine 1 and 2
+               wg.Wait()
+               if atomic.LoadInt32(n) != 2 {
+                       t.Fatal("Spurious wakeup from Wait")
+               }
+       }
+}
+
 func BenchmarkWaitGroupUncontended(b *testing.B) {
        type PaddedWaitGroup struct {
                WaitGroup