From 2dafda9164ef270b2558115c98529c652a669aeb Mon Sep 17 00:00:00 2001 From: greatroar <61184462+greatroar@users.noreply.github.com> Date: Tue, 25 Oct 2022 07:41:44 +0200 Subject: [PATCH] ui/progress: Load both values in a single Lock/Unlock We always need both values, except in a test, so we don't need to lock twice and risk scheduling in between. Also, removed the resetting in Done. This copied a mutex, which isn't allowed. Static analyzers tend to trip over that. --- internal/restic/find_test.go | 6 ++++-- internal/ui/progress/counter.go | 26 ++++++++++---------------- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/internal/restic/find_test.go b/internal/restic/find_test.go index b415501dc..3cbe95fb6 100644 --- a/internal/restic/find_test.go +++ b/internal/restic/find_test.go @@ -110,7 +110,8 @@ func TestFindUsedBlobs(t *testing.T) { continue } - test.Equals(t, p.Get(), uint64(i+1)) + v, _ := p.Get() + test.Equals(t, v, uint64(i+1)) goldenFilename := filepath.Join("testdata", fmt.Sprintf("used_blobs_snapshot%d", i)) want := loadIDSet(t, goldenFilename) @@ -151,7 +152,8 @@ func TestMultiFindUsedBlobs(t *testing.T) { for i := 1; i < 3; i++ { err := restic.FindUsedBlobs(context.TODO(), repo, snapshotTrees, usedBlobs, p) test.OK(t, err) - test.Equals(t, p.Get(), uint64(i*len(snapshotTrees))) + v, _ := p.Get() + test.Equals(t, v, uint64(i*len(snapshotTrees))) if !want.Equals(usedBlobs) { t.Errorf("wrong list of blobs returned:\n missing blobs: %v\n extra blobs: %v", diff --git a/internal/ui/progress/counter.go b/internal/ui/progress/counter.go index d2f75c9bf..90a09d0d8 100644 --- a/internal/ui/progress/counter.go +++ b/internal/ui/progress/counter.go @@ -78,32 +78,25 @@ func (c *Counter) Done() { c.tick.Stop() } close(c.stop) - <-c.stopped // Wait for last progress report. - *c = Counter{} // Prevent reuse. + <-c.stopped // Wait for last progress report. } -// Get the current Counter value. This method is concurrency-safe. -func (c *Counter) Get() uint64 { +// Get returns the current value and the maximum of c. +// This method is concurrency-safe. +func (c *Counter) Get() (v, max uint64) { c.valueMutex.Lock() - v := c.value + v, max = c.value, c.max c.valueMutex.Unlock() - return v -} - -func (c *Counter) getMax() uint64 { - c.valueMutex.Lock() - max := c.max - c.valueMutex.Unlock() - - return max + return v, max } func (c *Counter) run() { defer close(c.stopped) defer func() { // Must be a func so that time.Since isn't called at defer time. - c.report(c.Get(), c.getMax(), time.Since(c.start), true) + v, max := c.Get() + c.report(v, max, time.Since(c.start), true) }() var tick <-chan time.Time @@ -123,6 +116,7 @@ func (c *Counter) run() { return } - c.report(c.Get(), c.getMax(), now.Sub(c.start), false) + v, max := c.Get() + c.report(v, max, now.Sub(c.start), false) } }