Fix race in locker call to dec()

Can't safely use uint32 for locker since we need to decrement the count,
which requires loading the unit and doing some math, which is inherintly
racey.
Instead use Int32 which we can safely use with atomic and AddInt32 with
`-1`

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
This commit is contained in:
Brian Goff 2015-11-24 11:52:18 -05:00
parent 0fb39140d9
commit 058c4dfc3e
2 changed files with 39 additions and 5 deletions

View file

@ -32,22 +32,23 @@ type Locker struct {
type lockCtr struct { type lockCtr struct {
mu sync.Mutex mu sync.Mutex
// waiters is the number of waiters waiting to acquire the lock // waiters is the number of waiters waiting to acquire the lock
waiters uint32 // this is int32 instead of uint32 so we can add `-1` in `dec()`
waiters int32
} }
// inc increments the number of waiters waiting for the lock // inc increments the number of waiters waiting for the lock
func (l *lockCtr) inc() { func (l *lockCtr) inc() {
atomic.AddUint32(&l.waiters, 1) atomic.AddInt32(&l.waiters, 1)
} }
// dec decrements the number of waiters wating on the lock // dec decrements the number of waiters wating on the lock
func (l *lockCtr) dec() { func (l *lockCtr) dec() {
atomic.AddUint32(&l.waiters, ^uint32(l.waiters-1)) atomic.AddInt32(&l.waiters, -1)
} }
// count gets the current number of waiters // count gets the current number of waiters
func (l *lockCtr) count() uint32 { func (l *lockCtr) count() int32 {
return atomic.LoadUint32(&l.waiters) return atomic.LoadInt32(&l.waiters)
} }
// Lock locks the mutex // Lock locks the mutex

View file

@ -1,6 +1,7 @@
package locker package locker
import ( import (
"sync"
"testing" "testing"
"time" "time"
) )
@ -89,3 +90,35 @@ func TestLockerUnlock(t *testing.T) {
t.Fatalf("lock should not be blocked") t.Fatalf("lock should not be blocked")
} }
} }
func TestLockerConcurrency(t *testing.T) {
l := New()
var wg sync.WaitGroup
for i := 0; i <= 10000; i++ {
wg.Add(1)
go func() {
l.Lock("test")
// if there is a concurrency issue, will very likely panic here
l.Unlock("test")
wg.Done()
}()
}
chDone := make(chan struct{})
go func() {
wg.Wait()
close(chDone)
}()
select {
case <-chDone:
case <-time.After(10 * time.Second):
t.Fatal("timeout waiting for locks to complete")
}
// Since everything has unlocked this should not exist anymore
if ctr, exists := l.locks["test"]; exists {
t.Fatalf("lock should not exist: %v", ctr)
}
}