preserve context per request for local locks (#9828)

In the Current bug we were re-using the context
from previously granted lockers, this would
lead to lock timeouts for existing valid
read or write locks, leading to premature
timeout of locks.

This bug affects only local lockers in FS
or standalone erasure coded mode. This issue
is rather historical as well and was present
in lsync for some time but we were lucky to
not see it.

Similar changes are done in dsync as well
to keep the code more familiar

Fixes #9827
This commit is contained in:
Harshavardhana
2020-06-14 07:43:10 -07:00
committed by GitHub
parent 535efd34a0
commit d55f4336ae
6 changed files with 103 additions and 111 deletions

View File

@@ -32,12 +32,11 @@ type LRWMutex struct {
isWriteLock bool
ref int
m sync.Mutex // Mutex to prevent multiple simultaneous locks
ctx context.Context
}
// NewLRWMutex - initializes a new lsync RW mutex.
func NewLRWMutex(ctx context.Context) *LRWMutex {
return &LRWMutex{ctx: ctx}
func NewLRWMutex() *LRWMutex {
return &LRWMutex{}
}
// Lock holds a write lock on lm.
@@ -47,14 +46,14 @@ func NewLRWMutex(ctx context.Context) *LRWMutex {
func (lm *LRWMutex) Lock() {
const isWriteLock = true
lm.lockLoop(lm.id, lm.source, time.Duration(math.MaxInt64), isWriteLock)
lm.lockLoop(context.Background(), lm.id, lm.source, time.Duration(math.MaxInt64), isWriteLock)
}
// GetLock tries to get a write lock on lm before the timeout occurs.
func (lm *LRWMutex) GetLock(id string, source string, timeout time.Duration) (locked bool) {
func (lm *LRWMutex) GetLock(ctx context.Context, id string, source string, timeout time.Duration) (locked bool) {
const isWriteLock = true
return lm.lockLoop(id, source, timeout, isWriteLock)
return lm.lockLoop(ctx, id, source, timeout, isWriteLock)
}
// RLock holds a read lock on lm.
@@ -64,60 +63,55 @@ func (lm *LRWMutex) GetLock(id string, source string, timeout time.Duration) (lo
func (lm *LRWMutex) RLock() {
const isWriteLock = false
lm.lockLoop(lm.id, lm.source, time.Duration(1<<63-1), isWriteLock)
lm.lockLoop(context.Background(), lm.id, lm.source, time.Duration(1<<63-1), isWriteLock)
}
// GetRLock tries to get a read lock on lm before the timeout occurs.
func (lm *LRWMutex) GetRLock(id string, source string, timeout time.Duration) (locked bool) {
func (lm *LRWMutex) GetRLock(ctx context.Context, id string, source string, timeout time.Duration) (locked bool) {
const isWriteLock = false
return lm.lockLoop(id, source, timeout, isWriteLock)
return lm.lockLoop(ctx, id, source, timeout, isWriteLock)
}
func (lm *LRWMutex) lock(id, source string, isWriteLock bool) (locked bool) {
lm.m.Lock()
lm.id = id
lm.source = source
if isWriteLock {
if lm.ref == 0 && !lm.isWriteLock {
lm.ref = 1
lm.isWriteLock = true
locked = true
}
} else {
if !lm.isWriteLock {
lm.ref++
locked = true
}
}
lm.m.Unlock()
return locked
}
// lockLoop will acquire either a read or a write lock
//
// The call will block until the lock is granted using a built-in
// timing randomized back-off algorithm to try again until successful
func (lm *LRWMutex) lockLoop(id, source string, timeout time.Duration, isWriteLock bool) bool {
retryCtx, cancel := context.WithTimeout(lm.ctx, timeout)
func (lm *LRWMutex) lockLoop(ctx context.Context, id, source string, timeout time.Duration, isWriteLock bool) (locked bool) {
retryCtx, cancel := context.WithTimeout(ctx, timeout)
defer cancel()
// We timed out on the previous lock, incrementally wait
// for a longer back-off time and try again afterwards.
for range retry.NewTimer(retryCtx) {
// Try to acquire the lock.
var success bool
{
lm.m.Lock()
lm.id = id
lm.source = source
if isWriteLock {
if lm.ref == 0 && !lm.isWriteLock {
lm.ref = 1
lm.isWriteLock = true
success = true
}
} else {
if !lm.isWriteLock {
lm.ref++
success = true
}
}
lm.m.Unlock()
}
if success {
if lm.lock(id, source, isWriteLock) {
return true
}
}
// We timed out on the previous lock, incrementally wait
// for a longer back-off time and try again afterwards.
return false
}

View File

@@ -33,14 +33,15 @@ import (
func testSimpleWriteLock(t *testing.T, duration time.Duration) (locked bool) {
lrwm := NewLRWMutex(context.Background())
ctx := context.Background()
lrwm := NewLRWMutex()
if !lrwm.GetRLock("", "object1", time.Second) {
if !lrwm.GetRLock(ctx, "", "object1", time.Second) {
panic("Failed to acquire read lock")
}
// fmt.Println("1st read lock acquired, waiting...")
if !lrwm.GetRLock("", "object1", time.Second) {
if !lrwm.GetRLock(ctx, "", "object1", time.Second) {
panic("Failed to acquire read lock")
}
// fmt.Println("2nd read lock acquired, waiting...")
@@ -58,7 +59,7 @@ func testSimpleWriteLock(t *testing.T, duration time.Duration) (locked bool) {
}()
// fmt.Println("Trying to acquire write lock, waiting...")
locked = lrwm.GetLock("", "", duration)
locked = lrwm.GetLock(ctx, "", "", duration)
if locked {
// fmt.Println("Write lock acquired, waiting...")
time.Sleep(1 * time.Second)
@@ -90,10 +91,11 @@ func TestSimpleWriteLockTimedOut(t *testing.T) {
func testDualWriteLock(t *testing.T, duration time.Duration) (locked bool) {
lrwm := NewLRWMutex(context.Background())
ctx := context.Background()
lrwm := NewLRWMutex()
// fmt.Println("Getting initial write lock")
if !lrwm.GetLock("", "", time.Second) {
if !lrwm.GetLock(ctx, "", "", time.Second) {
panic("Failed to acquire initial write lock")
}
@@ -104,7 +106,7 @@ func testDualWriteLock(t *testing.T, duration time.Duration) (locked bool) {
}()
// fmt.Println("Trying to acquire 2nd write lock, waiting...")
locked = lrwm.GetLock("", "", duration)
locked = lrwm.GetLock(ctx, "", "", duration)
if locked {
// fmt.Println("2nd write lock acquired, waiting...")
time.Sleep(time.Second)
@@ -139,8 +141,8 @@ func TestDualWriteLockTimedOut(t *testing.T) {
// Test cases below are copied 1 to 1 from sync/rwmutex_test.go (adapted to use LRWMutex)
// Borrowed from rwmutex_test.go
func parallelReader(m *LRWMutex, clocked, cunlock, cdone chan bool) {
if m.GetRLock("", "", time.Second) {
func parallelReader(ctx context.Context, m *LRWMutex, clocked, cunlock, cdone chan bool) {
if m.GetRLock(ctx, "", "", time.Second) {
clocked <- true
<-cunlock
m.RUnlock()
@@ -151,13 +153,13 @@ func parallelReader(m *LRWMutex, clocked, cunlock, cdone chan bool) {
// Borrowed from rwmutex_test.go
func doTestParallelReaders(numReaders, gomaxprocs int) {
runtime.GOMAXPROCS(gomaxprocs)
m := NewLRWMutex(context.Background())
m := NewLRWMutex()
clocked := make(chan bool)
cunlock := make(chan bool)
cdone := make(chan bool)
for i := 0; i < numReaders; i++ {
go parallelReader(m, clocked, cunlock, cdone)
go parallelReader(context.Background(), m, clocked, cunlock, cdone)
}
// Wait for all parallel RLock()s to succeed.
for i := 0; i < numReaders; i++ {
@@ -183,7 +185,7 @@ func TestParallelReaders(t *testing.T) {
// Borrowed from rwmutex_test.go
func reader(rwm *LRWMutex, numIterations int, activity *int32, cdone chan bool) {
for i := 0; i < numIterations; i++ {
if rwm.GetRLock("", "", time.Second) {
if rwm.GetRLock(context.Background(), "", "", time.Second) {
n := atomic.AddInt32(activity, 1)
if n < 1 || n >= 10000 {
panic(fmt.Sprintf("wlock(%d)\n", n))
@@ -200,7 +202,7 @@ func reader(rwm *LRWMutex, numIterations int, activity *int32, cdone chan bool)
// Borrowed from rwmutex_test.go
func writer(rwm *LRWMutex, numIterations int, activity *int32, cdone chan bool) {
for i := 0; i < numIterations; i++ {
if rwm.GetLock("", "", time.Second) {
if rwm.GetLock(context.Background(), "", "", time.Second) {
n := atomic.AddInt32(activity, 10000)
if n != 10000 {
panic(fmt.Sprintf("wlock(%d)\n", n))
@@ -219,7 +221,7 @@ func HammerRWMutex(gomaxprocs, numReaders, numIterations int) {
runtime.GOMAXPROCS(gomaxprocs)
// Number of active readers + 10000 * number of active writers.
var activity int32
rwm := NewLRWMutex(context.Background())
rwm := NewLRWMutex()
cdone := make(chan bool)
go writer(rwm, numIterations, &activity, cdone)
var i int
@@ -257,7 +259,7 @@ func TestRWMutex(t *testing.T) {
// Borrowed from rwmutex_test.go
func TestDRLocker(t *testing.T) {
wl := NewLRWMutex(context.Background())
wl := NewLRWMutex()
var rl sync.Locker
wlocked := make(chan bool, 1)
rlocked := make(chan bool, 1)
@@ -298,7 +300,7 @@ func TestUnlockPanic(t *testing.T) {
t.Fatalf("unlock of unlocked RWMutex did not panic")
}
}()
mu := NewLRWMutex(context.Background())
mu := NewLRWMutex()
mu.Unlock()
}
@@ -309,7 +311,7 @@ func TestUnlockPanic2(t *testing.T) {
t.Fatalf("unlock of unlocked RWMutex did not panic")
}
}()
mu := NewLRWMutex(context.Background())
mu := NewLRWMutex()
mu.RLock()
mu.Unlock()
}
@@ -321,7 +323,7 @@ func TestRUnlockPanic(t *testing.T) {
t.Fatalf("read unlock of unlocked RWMutex did not panic")
}
}()
mu := NewLRWMutex(context.Background())
mu := NewLRWMutex()
mu.RUnlock()
}
@@ -332,7 +334,7 @@ func TestRUnlockPanic2(t *testing.T) {
t.Fatalf("read unlock of unlocked RWMutex did not panic")
}
}()
mu := NewLRWMutex(context.Background())
mu := NewLRWMutex()
mu.Lock()
mu.RUnlock()
}