From e9347c781be66056bbc724f4d70d4b8b9bc0288c Mon Sep 17 00:00:00 2001
From: Rui Ueyama <ruiu@google.com>
Date: Thu, 10 Apr 2014 18:44:44 +0400
Subject: [PATCH] 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
---
 src/pkg/sync/waitgroup.go      | 10 ++++++----
 src/pkg/sync/waitgroup_test.go | 26 ++++++++++++++++++++++++++
 2 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/src/pkg/sync/waitgroup.go b/src/pkg/sync/waitgroup.go
index 22681115cb..4c64dca393 100644
--- 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 0cbd51056a..4c0a043c01 100644
--- 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
-- 
2.50.0