accounting: add SetMaxCompletedTransfers method to fix bisync race #8815

Before this change bisync adjusted the global MaxCompletedTransfers
variable which caused races.

This adds a SetMaxCompletedTransfers method and uses it in bisync.

The MaxCompletedTransfers global becomes the default. This can be
changed externally if rclone is in use as a library, and the commit
history indicates that MaxCompletedTransfers was added for exactly
this purpose so we try not to break it here.

cmd/bisync/queue.go

@@ -246,9 +246,7 @@ func (b *bisyncRun) fastCopy(ctx context.Context, fsrc, fdst fs.Fs, files bilib.
 	}
 
 	b.SyncCI = fs.GetConfig(ctxCopy)                       // allows us to request graceful shutdown
-	if accounting.MaxCompletedTransfers != -1 {
+	accounting.Stats(ctxCopy).SetMaxCompletedTransfers(-1) // we need a complete list in the event of graceful shutdown
-		accounting.MaxCompletedTransfers = -1 // we need a complete list in the event of graceful shutdown
-	}
 	ctxCopy, b.CancelSync = context.WithCancel(ctxCopy)
 	b.testFn()
 	err := sync.Sync(ctxCopy, fdst, fsrc, b.opt.CreateEmptySrcDirs)

fs/accounting/stats.go

@@ -22,7 +22,10 @@ const (
 	averageStopAfter    = time.Minute
 )
 
-// MaxCompletedTransfers specifies maximum number of completed transfers in startedTransfers list
+// MaxCompletedTransfers specifies the default maximum number of
+// completed transfers in startedTransfers list. This can be adjusted
+// for a given StatsInfo by calling the SetMaxCompletedTransfers
+// method.
 var MaxCompletedTransfers = 100
 
 // StatsInfo accounts all transfers
@@ -64,6 +67,7 @@ type StatsInfo struct {
 	serverSideCopyBytes   int64
 	serverSideMoves       int64
 	serverSideMoveBytes   int64
+	maxCompletedTransfers int
 }
 
 type averageValues struct {
@@ -88,10 +92,19 @@ func NewStats(ctx context.Context) *StatsInfo {
 		inProgress:            newInProgress(ctx),
 		startTime:             time.Now(),
 		average:               averageValues{},
+		maxCompletedTransfers: MaxCompletedTransfers,
 	}
 	return s
 }
 
+// SetMaxCompletedTransfers sets the maximum number of completed transfers to keep.
+func (s *StatsInfo) SetMaxCompletedTransfers(n int) *StatsInfo {
+	s.mu.Lock()
+	s.maxCompletedTransfers = n
+	s.mu.Unlock()
+	return s
+}
+
 // RemoteStats returns stats for rc
 //
 // If short is true then the transfers and checkers won't be added.
@@ -914,13 +927,14 @@ func (s *StatsInfo) RemoveTransfer(transfer *Transfer) {
 // PruneTransfers makes sure there aren't too many old transfers by removing
 // a single finished transfer. Returns true if it removed a transfer.
 func (s *StatsInfo) PruneTransfers() bool {
-	if MaxCompletedTransfers < 0 {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	if s.maxCompletedTransfers < 0 {
 		return false
 	}
 	removed := false
-	s.mu.Lock()
 	// remove a transfer from the start if we are over quota
-	if len(s.startedTransfers) > MaxCompletedTransfers+s.ci.Transfers {
+	if len(s.startedTransfers) > s.maxCompletedTransfers+s.ci.Transfers {
 		for i, tr := range s.startedTransfers {
 			if tr.IsDone() {
 				s._removeTransfer(tr, i)
@@ -929,7 +943,6 @@ func (s *StatsInfo) PruneTransfers() bool {
 			}
 		}
 	}
-	s.mu.Unlock()
 	return removed
 }