lock: Synchronize Refresh() and Stale()

The lock test creates a lock and checks that it is not stale. However,
it is possible that the lock is refreshed concurrently, which updates
the lock timestamp. Checking the timestamp in `Stale()` without
synchronization results in a data race. Thus add a lock to prevent
concurrent accesses.
This commit is contained in:
Michael Eischer 2022-11-11 21:52:53 +01:00
parent e1ba7ab684
commit 13fbc96ed3

View file

@ -26,6 +26,7 @@ import (
// A lock must be refreshed regularly to not be considered stale, this must be // A lock must be refreshed regularly to not be considered stale, this must be
// triggered by regularly calling Refresh. // triggered by regularly calling Refresh.
type Lock struct { type Lock struct {
lock sync.Mutex
Time time.Time `json:"time"` Time time.Time `json:"time"`
Exclusive bool `json:"exclusive"` Exclusive bool `json:"exclusive"`
Hostname string `json:"hostname"` Hostname string `json:"hostname"`
@ -195,6 +196,8 @@ var StaleLockTimeout = 30 * time.Minute
// older than 30 minutes or if it was created on the current machine and the // older than 30 minutes or if it was created on the current machine and the
// process isn't alive any more. // process isn't alive any more.
func (l *Lock) Stale() bool { func (l *Lock) Stale() bool {
l.lock.Lock()
defer l.lock.Unlock()
debug.Log("testing if lock %v for process %d is stale", l, l.PID) debug.Log("testing if lock %v for process %d is stale", l, l.PID)
if time.Since(l.Time) > StaleLockTimeout { if time.Since(l.Time) > StaleLockTimeout {
debug.Log("lock is stale, timestamp is too old: %v\n", l.Time) debug.Log("lock is stale, timestamp is too old: %v\n", l.Time)
@ -229,12 +232,17 @@ func (l *Lock) Stale() bool {
// timestamp. Afterwards the old lock is removed. // timestamp. Afterwards the old lock is removed.
func (l *Lock) Refresh(ctx context.Context) error { func (l *Lock) Refresh(ctx context.Context) error {
debug.Log("refreshing lock %v", l.lockID) debug.Log("refreshing lock %v", l.lockID)
l.lock.Lock()
l.Time = time.Now() l.Time = time.Now()
l.lock.Unlock()
id, err := l.createLock(ctx) id, err := l.createLock(ctx)
if err != nil { if err != nil {
return err return err
} }
l.lock.Lock()
defer l.lock.Unlock()
debug.Log("new lock ID %v", id) debug.Log("new lock ID %v", id)
oldLockID := l.lockID oldLockID := l.lockID
l.lockID = &id l.lockID = &id
@ -242,7 +250,10 @@ func (l *Lock) Refresh(ctx context.Context) error {
return l.repo.Backend().Remove(context.TODO(), Handle{Type: LockFile, Name: oldLockID.String()}) return l.repo.Backend().Remove(context.TODO(), Handle{Type: LockFile, Name: oldLockID.String()})
} }
func (l Lock) String() string { func (l *Lock) String() string {
l.lock.Lock()
defer l.lock.Unlock()
text := fmt.Sprintf("PID %d on %s by %s (UID %d, GID %d)\nlock was created at %s (%s ago)\nstorage ID %v", text := fmt.Sprintf("PID %d on %s by %s (UID %d, GID %d)\nlock was created at %s (%s ago)\nstorage ID %v",
l.PID, l.Hostname, l.Username, l.UID, l.GID, l.PID, l.Hostname, l.Username, l.UID, l.GID,
l.Time.Format("2006-01-02 15:04:05"), time.Since(l.Time), l.Time.Format("2006-01-02 15:04:05"), time.Since(l.Time),