diff --git a/changelog/unreleased/issue-2452 b/changelog/unreleased/issue-2452 new file mode 100644 index 000000000..3a82e9b1b --- /dev/null +++ b/changelog/unreleased/issue-2452 @@ -0,0 +1,11 @@ +Bugfix: Improve error handling of repository locking + +When the lock refresh failed to delete the old lock file, it forgot about the +newly created one. Instead it continued trying to delete the old (usually no +longer existing) lock file and thus over time lots of lock files accumulated. +This has been fixed. + +https://github.com/restic/restic/issues/2452 +https://github.com/restic/restic/issues/2473 +https://github.com/restic/restic/issues/2562 +https://github.com/restic/restic/pull/3512 diff --git a/cmd/restic/global.go b/cmd/restic/global.go index 9f42f851a..e3b4020d6 100644 --- a/cmd/restic/global.go +++ b/cmd/restic/global.go @@ -98,6 +98,8 @@ func init() { var cancel context.CancelFunc globalOptions.ctx, cancel = context.WithCancel(context.Background()) AddCleanupHandler(func() error { + // Must be called before the unlock cleanup handler to ensure that the latter is + // not blocked due to limited number of backend connections, see #1434 cancel() return nil }) diff --git a/cmd/restic/lock.go b/cmd/restic/lock.go index 822059943..64f82cf52 100644 --- a/cmd/restic/lock.go +++ b/cmd/restic/lock.go @@ -16,6 +16,7 @@ var globalLocks struct { cancelRefresh chan struct{} refreshWG sync.WaitGroup sync.Mutex + sync.Once } func lockRepo(ctx context.Context, repo *repository.Repository) (*restic.Lock, error) { @@ -27,6 +28,12 @@ func lockRepoExclusive(ctx context.Context, repo *repository.Repository) (*resti } func lockRepository(ctx context.Context, repo *repository.Repository, exclusive bool) (*restic.Lock, error) { + // make sure that a repository is unlocked properly and after cancel() was + // called by the cleanup handler in global.go + globalLocks.Do(func() { + AddCleanupHandler(unlockAll) + }) + lockFn := restic.NewLock if exclusive { lockFn = restic.NewExclusiveLock @@ -128,7 +135,3 @@ func unlockAll() error { return nil } - -func init() { - AddCleanupHandler(unlockAll) -} diff --git a/internal/restic/lock.go b/internal/restic/lock.go index e79882f80..9a5fd841d 100644 --- a/internal/restic/lock.go +++ b/internal/restic/lock.go @@ -223,15 +223,11 @@ func (l *Lock) Refresh(ctx context.Context) error { return err } - err = l.repo.Backend().Remove(context.TODO(), Handle{Type: LockFile, Name: l.lockID.String()}) - if err != nil { - return err - } - debug.Log("new lock ID %v", id) + oldLockID := l.lockID l.lockID = &id - return nil + return l.repo.Backend().Remove(context.TODO(), Handle{Type: LockFile, Name: oldLockID.String()}) } func (l Lock) String() string {