From 637c1cfb6629f57fa5621284dd0047937072baba Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sun, 7 Jun 2026 13:06:34 +0200 Subject: [PATCH] repository: unexport internal lock file helpers Unexport UnlockCancelDelay, IsInvalidLock, ErrRemovedLock, NewLock, StaleLockTimeout and ForAllLocks --- internal/repository/lock.go | 10 +++---- internal/repository/lock_file.go | 39 +++++++++++++-------------- internal/repository/lock_file_test.go | 28 +++++++++---------- 3 files changed, 38 insertions(+), 39 deletions(-) diff --git a/internal/repository/lock.go b/internal/repository/lock.go index 488aca022..a91341f42 100644 --- a/internal/repository/lock.go +++ b/internal/repository/lock.go @@ -34,7 +34,7 @@ var lockerInst = &locker{ refreshInterval: defaultRefreshInterval, // consider a lock refresh failed a bit before the lock actually becomes stale // the difference allows to compensate for a small time drift between clients. - refreshabilityTimeout: StaleLockTimeout - defaultRefreshInterval*3/2, + refreshabilityTimeout: staleLockTimeout - defaultRefreshInterval*3/2, } func LockRepo(ctx context.Context, repo *Repository, exclusive bool, retryLock time.Duration, printRetry func(msg string), logger func(format string, args ...interface{})) (Unlocker, context.Context, error) { @@ -55,7 +55,7 @@ func (l *locker) Lock(ctx context.Context, r *Repository, exclusive bool, retryL retryLoop: for { - lock, err = NewLock(ctx, repo, exclusive) + lock, err = newLock(ctx, repo, exclusive) if err != nil && IsAlreadyLocked(err) { if !retryMessagePrinted { @@ -72,7 +72,7 @@ retryLoop: case <-retryTimeout: debug.Log("repo already locked, timeout expired") // Last lock attempt - lock, err = NewLock(ctx, repo, exclusive) + lock, err = newLock(ctx, repo, exclusive) break retryLoop case <-retrySleepCh: retrySleep = minDuration(retrySleep*2, l.retrySleepMax) @@ -82,7 +82,7 @@ retryLoop: break retryLoop } } - if IsInvalidLock(err) { + if isInvalidLock(err) { return nil, ctx, errors.Fatalf("%v\n\nthe `unlock --remove-all` command can be used to remove invalid locks. Make sure that no other restic process is accessing the repository when running the command", err) } if err != nil { @@ -277,7 +277,7 @@ func (l *unlocker) Unlock() { // RemoveStaleLocks deletes all locks detected as stale from the repository. func RemoveStaleLocks(ctx context.Context, repo *Repository) (uint, error) { var processed uint - err := ForAllLocks(ctx, repo, nil, func(id restic.ID, lock *Lock, err error) error { + err := forAllLocks(ctx, repo, nil, func(id restic.ID, lock *Lock, err error) error { if err != nil { // ignore locks that cannot be loaded debug.Log("ignore lock %v: %v", id, err) diff --git a/internal/repository/lock_file.go b/internal/repository/lock_file.go index b0a0b3ede..78d60979f 100644 --- a/internal/repository/lock_file.go +++ b/internal/repository/lock_file.go @@ -16,9 +16,9 @@ import ( "github.com/restic/restic/internal/restic" ) -// UnlockCancelDelay bounds the duration how long lock cleanup operations will wait +// unlockCancelDelay bounds the duration how long lock cleanup operations will wait // if the passed in context was canceled. -const UnlockCancelDelay = 1 * time.Minute +const unlockCancelDelay = 1 * time.Minute // Lock represents a process locking the repository for an operation. // @@ -63,8 +63,7 @@ func IsAlreadyLocked(err error) bool { return errors.As(err, &e) } -// invalidLockError is returned when NewLock or NewExclusiveLock fail due -// to an invalid lock. +// invalidLockError is returned when newLock fails due to an invalid lock. type invalidLockError struct { err error } @@ -77,14 +76,14 @@ func (e *invalidLockError) Unwrap() error { return e.err } -// IsInvalidLock returns true iff err indicates that locking failed due to +// isInvalidLock returns true iff err indicates that locking failed due to // an invalid lock. -func IsInvalidLock(err error) bool { +func isInvalidLock(err error) bool { var e *invalidLockError return errors.As(err, &e) } -var ErrRemovedLock = errors.New("lock file was removed in the meantime") +var errRemovedLock = errors.New("lock file was removed in the meantime") var waitBeforeLockCheck = 200 * time.Millisecond @@ -98,11 +97,11 @@ func TestSetLockTimeout(t testing.TB, d time.Duration) { initialWaitBetweenLockRetries = d } -// NewLock returns a new lock for the repository. If an +// newLock returns a new lock for the repository. If an // exclusive lock is already held by another process, it returns an error -// that satisfies IsAlreadyLocked. If the new lock is exclude, then other +// that satisfies IsAlreadyLocked. If the new lock is exclusive, then other // non-exclusive locks also result in an IsAlreadyLocked error. -func NewLock(ctx context.Context, repo restic.Unpacked[restic.FileType], exclusive bool) (*Lock, error) { +func newLock(ctx context.Context, repo restic.Unpacked[restic.FileType], exclusive bool) (*Lock, error) { lock := &Lock{ Time: time.Now(), PID: os.Getpid(), @@ -177,7 +176,7 @@ func (l *Lock) checkForOtherLocks(ctx context.Context) error { // Store updates in new IDSet to prevent data races var m sync.Mutex newCheckedIDs := checkedIDs.Clone() - err = ForAllLocks(ctx, l.repo, checkedIDs, func(id restic.ID, lock *Lock, err error) error { + err = forAllLocks(ctx, l.repo, checkedIDs, func(id restic.ID, lock *Lock, err error) error { if err != nil { // if we cannot load a lock then it is unclear whether it can be ignored // it could either be invalid or just unreadable due to network/permission problems @@ -243,13 +242,13 @@ func (l *Lock) Unlock(ctx context.Context) error { return nil } - ctx, cancel := delayedCancelContext(ctx, UnlockCancelDelay) + ctx, cancel := delayedCancelContext(ctx, unlockCancelDelay) defer cancel() return l.repo.RemoveUnpacked(ctx, restic.LockFile, *l.lockID) } -var StaleLockTimeout = 30 * time.Minute +var staleLockTimeout = 30 * time.Minute // Stale returns true if the lock is stale. A lock is stale if the timestamp is // older than 30 minutes or if it was created on the current machine and the @@ -258,7 +257,7 @@ func (l *Lock) Stale() bool { l.lock.Lock() defer l.lock.Unlock() debug.Log("testing if lock %v for process %d is stale", l.lockID, 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) return true } @@ -323,7 +322,7 @@ func (l *Lock) Refresh(ctx context.Context) error { oldLockID := l.lockID l.lockID = &id - ctx, cancel := delayedCancelContext(ctx, UnlockCancelDelay) + ctx, cancel := delayedCancelContext(ctx, unlockCancelDelay) defer cancel() return l.repo.RemoveUnpacked(ctx, restic.LockFile, *oldLockID) @@ -339,7 +338,7 @@ func (l *Lock) RefreshStaleLock(ctx context.Context) error { if err != nil { return err } else if !exists { - return ErrRemovedLock + return errRemovedLock } l.lock.Lock() @@ -354,7 +353,7 @@ func (l *Lock) RefreshStaleLock(ctx context.Context) error { exists, err = l.checkExistence(ctx) - ctx, cancel := delayedCancelContext(ctx, UnlockCancelDelay) + ctx, cancel := delayedCancelContext(ctx, unlockCancelDelay) defer cancel() if err != nil { @@ -366,7 +365,7 @@ func (l *Lock) RefreshStaleLock(ctx context.Context) error { if !exists { // cleanup replacement lock _ = l.repo.RemoveUnpacked(ctx, restic.LockFile, id) - return ErrRemovedLock + return errRemovedLock } l.lock.Lock() @@ -433,11 +432,11 @@ func LoadLock(ctx context.Context, repo restic.LoaderUnpacked, id restic.ID) (*L return lock, nil } -// ForAllLocks reads all locks in parallel and calls the given callback. +// forAllLocks reads all locks in parallel and calls the given callback. // It is guaranteed that the function is not run concurrently. If the // callback returns an error, this function is cancelled and also returns that error. // If a lock ID is passed via excludeID, it will be ignored. -func ForAllLocks(ctx context.Context, repo restic.ListerLoaderUnpacked, excludeIDs restic.IDSet, fn func(restic.ID, *Lock, error) error) error { +func forAllLocks(ctx context.Context, repo restic.ListerLoaderUnpacked, excludeIDs restic.IDSet, fn func(restic.ID, *Lock, error) error) error { var m sync.Mutex // For locks decoding is nearly for free, thus just assume were only limited by IO diff --git a/internal/repository/lock_file_test.go b/internal/repository/lock_file_test.go index 5443c3ca7..1e2256860 100644 --- a/internal/repository/lock_file_test.go +++ b/internal/repository/lock_file_test.go @@ -18,7 +18,7 @@ func TestLockFile(t *testing.T) { repo := TestRepository(t) TestSetLockTimeout(t, 5*time.Millisecond) - lock, err := NewLock(context.TODO(), &internalRepository{repo}, false) + lock, err := newLock(context.TODO(), &internalRepository{repo}, false) rtest.OK(t, err) rtest.OK(t, lock.Unlock(context.TODO())) @@ -28,7 +28,7 @@ func TestDoubleUnlock(t *testing.T) { repo := TestRepository(t) TestSetLockTimeout(t, 5*time.Millisecond) - lock, err := NewLock(context.TODO(), &internalRepository{repo}, false) + lock, err := newLock(context.TODO(), &internalRepository{repo}, false) rtest.OK(t, err) rtest.OK(t, lock.Unlock(context.TODO())) @@ -42,10 +42,10 @@ func TestMultipleLock(t *testing.T) { repo := TestRepository(t) TestSetLockTimeout(t, 5*time.Millisecond) - lock1, err := NewLock(context.TODO(), &internalRepository{repo}, false) + lock1, err := newLock(context.TODO(), &internalRepository{repo}, false) rtest.OK(t, err) - lock2, err := NewLock(context.TODO(), &internalRepository{repo}, false) + lock2, err := newLock(context.TODO(), &internalRepository{repo}, false) rtest.OK(t, err) rtest.OK(t, lock1.Unlock(context.TODO())) @@ -68,10 +68,10 @@ func TestMultipleLockFailure(t *testing.T) { repo, _ := TestRepositoryWithBackend(t, be, 0, Options{}) TestSetLockTimeout(t, 5*time.Millisecond) - lock1, err := NewLock(context.TODO(), &internalRepository{repo}, false) + lock1, err := newLock(context.TODO(), &internalRepository{repo}, false) rtest.OK(t, err) - _, err = NewLock(context.TODO(), &internalRepository{repo}, false) + _, err = newLock(context.TODO(), &internalRepository{repo}, false) rtest.Assert(t, err != nil, "unreadable lock file did not result in an error") rtest.OK(t, lock1.Unlock(context.TODO())) @@ -80,7 +80,7 @@ func TestMultipleLockFailure(t *testing.T) { func TestLockExclusive(t *testing.T) { repo := TestRepository(t) - elock, err := NewLock(context.TODO(), &internalRepository{repo}, true) + elock, err := newLock(context.TODO(), &internalRepository{repo}, true) rtest.OK(t, err) rtest.OK(t, elock.Unlock(context.TODO())) } @@ -89,10 +89,10 @@ func TestLockOnExclusiveLockedRepo(t *testing.T) { repo := TestRepository(t) TestSetLockTimeout(t, 5*time.Millisecond) - elock, err := NewLock(context.TODO(), &internalRepository{repo}, true) + elock, err := newLock(context.TODO(), &internalRepository{repo}, true) rtest.OK(t, err) - lock, err := NewLock(context.TODO(), &internalRepository{repo}, false) + lock, err := newLock(context.TODO(), &internalRepository{repo}, false) rtest.Assert(t, err != nil, "create normal lock with exclusively locked repo didn't return an error") rtest.Assert(t, IsAlreadyLocked(err), @@ -106,10 +106,10 @@ func TestExclusiveLockOnLockedRepo(t *testing.T) { repo := TestRepository(t) TestSetLockTimeout(t, 5*time.Millisecond) - elock, err := NewLock(context.TODO(), &internalRepository{repo}, false) + elock, err := newLock(context.TODO(), &internalRepository{repo}, false) rtest.OK(t, err) - lock, err := NewLock(context.TODO(), &internalRepository{repo}, true) + lock, err := newLock(context.TODO(), &internalRepository{repo}, true) rtest.Assert(t, err != nil, "create normal lock with exclusively locked repo didn't return an error") rtest.Assert(t, IsAlreadyLocked(err), @@ -198,7 +198,7 @@ func testLockRefresh(t *testing.T, refresh func(lock *Lock) error) { repo := TestRepository(t) TestSetLockTimeout(t, 5*time.Millisecond) - lock, err := NewLock(context.TODO(), &internalRepository{repo}, false) + lock, err := newLock(context.TODO(), &internalRepository{repo}, false) rtest.OK(t, err) time0 := lock.Time @@ -234,7 +234,7 @@ func TestLockRefreshStaleMissing(t *testing.T) { repo, _, be := TestRepositoryWithVersion(t, 0) TestSetLockTimeout(t, 5*time.Millisecond) - lock, err := NewLock(context.TODO(), &internalRepository{repo}, false) + lock, err := newLock(context.TODO(), &internalRepository{repo}, false) rtest.OK(t, err) lockID := checkSingleLock(t, repo) @@ -242,5 +242,5 @@ func TestLockRefreshStaleMissing(t *testing.T) { rtest.OK(t, be.Remove(context.TODO(), backend.Handle{Type: restic.LockFile, Name: lockID.String()})) time.Sleep(time.Millisecond) err = lock.RefreshStaleLock(context.TODO()) - rtest.Assert(t, err == ErrRemovedLock, "unexpected error, expected %v, got %v", ErrRemovedLock, err) + rtest.Assert(t, err == errRemovedLock, "unexpected error, expected %v, got %v", errRemovedLock, err) }