Merge pull request #3512 from MichaelEischer/cleaner-lock-refresh

Prevent lock refresh from leaving behind lots of stale locks

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

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
 	})

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)
-}

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 {