From cf9b973fe4bb67aaaecfae05b2c4e248d8eb5539 Mon Sep 17 00:00:00 2001
From: Nick Craig-Wood <nick@craig-wood.com>
Date: Tue, 3 Sep 2019 11:57:56 +0100
Subject: [PATCH] accounting: fix locking in Transfer to avoid deadlock with
 --progress

Before this change, using -P occasionally deadlocked on the Transfer
mutex when Transfer.Done() was called with a non nil error and the
StatsInfo mutex since they mutually call each other.

This was fixed by making sure that the Transfer mutex is always
released before calling any StatsInfo methods.

This improves on: 6f87267b347b4935

Fixes #3505
---
 fs/accounting/transfer.go | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/fs/accounting/transfer.go b/fs/accounting/transfer.go
index e6b6d8d6a..0d968661b 100644
--- a/fs/accounting/transfer.go
+++ b/fs/accounting/transfer.go
@@ -48,6 +48,10 @@ type Transfer struct {
 	checking  bool
 
 	// Protects all below
+	//
+	// NB to avoid deadlocks we must release this lock before
+	// calling any methods on Transfer.stats.  This is because
+	// StatsInfo calls back into Transfer.
 	mu          sync.RWMutex
 	acc         *Account
 	err         error
@@ -79,20 +83,26 @@ func newTransferRemoteSize(stats *StatsInfo, remote string, size int64, checking
 // Done ends the transfer.
 // Must be called after transfer is finished to run proper cleanups.
 func (tr *Transfer) Done(err error) {
-	tr.mu.Lock()
-
 	if err != nil {
 		tr.stats.Error(err)
+
+		tr.mu.Lock()
 		tr.err = err
+		tr.mu.Unlock()
 	}
-	if tr.acc != nil {
-		if err := tr.acc.Close(); err != nil {
+
+	tr.mu.RLock()
+	acc := tr.acc
+	tr.mu.RUnlock()
+
+	if acc != nil {
+		if err := acc.Close(); err != nil {
 			fs.LogLevelPrintf(fs.Config.StatsLogLevel, nil, "can't close account: %+v\n", err)
 		}
 	}
 
+	tr.mu.Lock()
 	tr.completedAt = time.Now()
-
 	tr.mu.Unlock()
 
 	if tr.checking {