diff --git a/fs/accounting/stats.go b/fs/accounting/stats.go index c666e39ff..8eabb12cc 100644 --- a/fs/accounting/stats.go +++ b/fs/accounting/stats.go @@ -22,14 +22,14 @@ func init() { // StatsInfo accounts all transfers type StatsInfo struct { - lock sync.RWMutex + mu sync.RWMutex bytes int64 errors int64 lastError error checks int64 - checking stringSet + checking *stringSet transfers int64 - transferring stringSet + transferring *stringSet deletes int64 start time.Time inProgress *inProgress @@ -38,8 +38,8 @@ type StatsInfo struct { // NewStats cretates an initialised StatsInfo func NewStats() *StatsInfo { return &StatsInfo{ - checking: make(stringSet, fs.Config.Checkers), - transferring: make(stringSet, fs.Config.Transfers), + checking: newStringSet(fs.Config.Checkers), + transferring: newStringSet(fs.Config.Transfers), start: time.Now(), inProgress: newInProgress(), } @@ -47,8 +47,8 @@ func NewStats() *StatsInfo { // String convert the StatsInfo to a string for printing func (s *StatsInfo) String() string { - s.lock.RLock() - defer s.lock.RUnlock() + s.mu.RLock() + dt := time.Now().Sub(s.start) dtSeconds := dt.Seconds() speed := 0.0 @@ -74,10 +74,15 @@ Elapsed time: %10v s.checks, s.transfers, dtRounded) - if len(s.checking) > 0 { + + // checking and transferring have their own locking so unlock + // here to prevent deadlock on GetBytes + s.mu.RUnlock() + + if !s.checking.empty() { fmt.Fprintf(buf, "Checking:\n%s\n", s.checking) } - if len(s.transferring) > 0 { + if !s.transferring.empty() { fmt.Fprintf(buf, "Transferring:\n%s\n", s.transferring) } return buf.String() @@ -90,51 +95,51 @@ func (s *StatsInfo) Log() { // Bytes updates the stats for bytes bytes func (s *StatsInfo) Bytes(bytes int64) { - s.lock.Lock() - defer s.lock.Unlock() + s.mu.Lock() + defer s.mu.Unlock() s.bytes += bytes } // GetBytes returns the number of bytes transferred so far func (s *StatsInfo) GetBytes() int64 { - s.lock.Lock() - defer s.lock.Unlock() + s.mu.Lock() + defer s.mu.Unlock() return s.bytes } // Errors updates the stats for errors func (s *StatsInfo) Errors(errors int64) { - s.lock.Lock() - defer s.lock.Unlock() + s.mu.Lock() + defer s.mu.Unlock() s.errors += errors } // GetErrors reads the number of errors func (s *StatsInfo) GetErrors() int64 { - s.lock.RLock() - defer s.lock.RUnlock() + s.mu.RLock() + defer s.mu.RUnlock() return s.errors } // GetLastError returns the lastError func (s *StatsInfo) GetLastError() error { - s.lock.RLock() - defer s.lock.RUnlock() + s.mu.RLock() + defer s.mu.RUnlock() return s.lastError } // Deletes updates the stats for deletes func (s *StatsInfo) Deletes(deletes int64) int64 { - s.lock.Lock() - defer s.lock.Unlock() + s.mu.Lock() + defer s.mu.Unlock() s.deletes += deletes return s.deletes } // ResetCounters sets the counters (bytes, checks, errors, transfers) to 0 func (s *StatsInfo) ResetCounters() { - s.lock.RLock() - defer s.lock.RUnlock() + s.mu.RLock() + defer s.mu.RUnlock() s.bytes = 0 s.errors = 0 s.checks = 0 @@ -144,63 +149,59 @@ func (s *StatsInfo) ResetCounters() { // ResetErrors sets the errors count to 0 func (s *StatsInfo) ResetErrors() { - s.lock.RLock() - defer s.lock.RUnlock() + s.mu.RLock() + defer s.mu.RUnlock() s.errors = 0 } // Errored returns whether there have been any errors func (s *StatsInfo) Errored() bool { - s.lock.RLock() - defer s.lock.RUnlock() + s.mu.RLock() + defer s.mu.RUnlock() return s.errors != 0 } // Error adds a single error into the stats and assigns lastError func (s *StatsInfo) Error(err error) { - s.lock.Lock() - defer s.lock.Unlock() + s.mu.Lock() + defer s.mu.Unlock() s.errors++ s.lastError = err } // Checking adds a check into the stats func (s *StatsInfo) Checking(remote string) { - s.lock.Lock() - defer s.lock.Unlock() - s.checking[remote] = struct{}{} + s.checking.add(remote) } // DoneChecking removes a check from the stats func (s *StatsInfo) DoneChecking(remote string) { - s.lock.Lock() - defer s.lock.Unlock() - delete(s.checking, remote) + s.checking.del(remote) + s.mu.Lock() s.checks++ + s.mu.Unlock() } // GetTransfers reads the number of transfers func (s *StatsInfo) GetTransfers() int64 { - s.lock.RLock() - defer s.lock.RUnlock() + s.mu.RLock() + defer s.mu.RUnlock() return s.transfers } // Transferring adds a transfer into the stats func (s *StatsInfo) Transferring(remote string) { - s.lock.Lock() - defer s.lock.Unlock() - s.transferring[remote] = struct{}{} + s.transferring.add(remote) } // DoneTransferring removes a transfer from the stats // // if ok is true then it increments the transfers count func (s *StatsInfo) DoneTransferring(remote string, ok bool) { - s.lock.Lock() - defer s.lock.Unlock() - delete(s.transferring, remote) + s.transferring.del(remote) if ok { + s.mu.Lock() s.transfers++ + s.mu.Unlock() } } diff --git a/fs/accounting/stringset.go b/fs/accounting/stringset.go index 09b397ab5..2b18c9df7 100644 --- a/fs/accounting/stringset.go +++ b/fs/accounting/stringset.go @@ -3,15 +3,49 @@ package accounting import ( "sort" "strings" + "sync" ) // stringSet holds a set of strings -type stringSet map[string]struct{} +type stringSet struct { + mu sync.RWMutex + items map[string]struct{} +} + +// newStringSet creates a new empty string set of capacity size +func newStringSet(size int) *stringSet { + return &stringSet{ + items: make(map[string]struct{}, size), + } +} + +// add adds remote to the set +func (ss *stringSet) add(remote string) { + ss.mu.Lock() + ss.items[remote] = struct{}{} + ss.mu.Unlock() +} + +// del removes remote from the set +func (ss *stringSet) del(remote string) { + ss.mu.Lock() + delete(ss.items, remote) + ss.mu.Unlock() +} + +// empty returns whether the set has any items +func (ss *stringSet) empty() bool { + ss.mu.RLock() + defer ss.mu.RUnlock() + return len(ss.items) == 0 +} // Strings returns all the strings in the stringSet -func (ss stringSet) Strings() []string { - strings := make([]string, 0, len(ss)) - for name := range ss { +func (ss *stringSet) Strings() []string { + ss.mu.RLock() + defer ss.mu.RUnlock() + strings := make([]string, 0, len(ss.items)) + for name := range ss.items { var out string if acc := Stats.inProgress.get(name); acc != nil { out = acc.String() @@ -26,6 +60,6 @@ func (ss stringSet) Strings() []string { } // String returns all the file names in the stringSet joined by newline -func (ss stringSet) String() string { +func (ss *stringSet) String() string { return strings.Join(ss.Strings(), "\n") }