From eb10ac346f7ee91da6b823738704c6a0c57b673d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Matczuk?= Date: Fri, 3 Jan 2020 14:55:30 +0100 Subject: [PATCH] fs/accounting: Added StatsInfo locking in statsGroups sum function (#3844) Without the fix we can have a race, example: ``` Write at 0x00c000432039 by goroutine 187: github.com/rclone/rclone/fs/accounting.(*StatsInfo).Error() fs/accounting/stats.go:495 +0x3f1 github.com/rclone/rclone/fs/accounting.(*StatsInfo).Error-fm() fs/accounting/stats.go:477 +0x55 github.com/rclone/rclone/fs/walk.listRwalk.func1() fs/walk/walk.go:162 +0xd2 github.com/rclone/rclone/fs/walk.walk.func2() fs/walk/walk.go:402 +0x30f Previous read at 0x00c000432039 by goroutine 184: github.com/rclone/rclone/fs/accounting.(*statsGroups).sum() fs/accounting/stats_groups.go:351 +0xcae github.com/rclone/rclone/fs/accounting.rcTransferredStats() fs/accounting/stats_groups.go:132 +0x1f4 ``` Fixes #3844 --- fs/accounting/stats_groups.go | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/fs/accounting/stats_groups.go b/fs/accounting/stats_groups.go index c96061052..35b750bd6 100644 --- a/fs/accounting/stats_groups.go +++ b/fs/accounting/stats_groups.go @@ -344,22 +344,27 @@ func (sg *statsGroups) names() []string { func (sg *statsGroups) sum() *StatsInfo { sg.mu.Lock() defer sg.mu.Unlock() + sum := NewStats() for _, stats := range sg.m { - sum.bytes += stats.bytes - sum.errors += stats.errors - sum.fatalError = sum.fatalError || stats.fatalError - sum.retryError = sum.retryError || stats.retryError - sum.checks += stats.checks - sum.transfers += stats.transfers - sum.deletes += stats.deletes - sum.checking.merge(stats.checking) - sum.transferring.merge(stats.transferring) - sum.inProgress.merge(stats.inProgress) - if sum.lastError == nil && stats.lastError != nil { - sum.lastError = stats.lastError + stats.mu.RLock() + { + sum.bytes += stats.bytes + sum.errors += stats.errors + sum.fatalError = sum.fatalError || stats.fatalError + sum.retryError = sum.retryError || stats.retryError + sum.checks += stats.checks + sum.transfers += stats.transfers + sum.deletes += stats.deletes + sum.checking.merge(stats.checking) + sum.transferring.merge(stats.transferring) + sum.inProgress.merge(stats.inProgress) + if sum.lastError == nil && stats.lastError != nil { + sum.lastError = stats.lastError + } + sum.startedTransfers = append(sum.startedTransfers, stats.startedTransfers...) } - sum.startedTransfers = append(sum.startedTransfers, stats.startedTransfers...) + stats.mu.RUnlock() } return sum }