Commit e9347c78 authored by Rui Ueyama's avatar Rui Ueyama Committed by Dmitriy Vyukov

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
parent 6278a954
......@@ -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()
}
......
......@@ -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
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment