From 0a1ce4f2078701a820befc9011b0726f0abd7f59 Mon Sep 17 00:00:00 2001 From: greatroar <61184462+greatroar@users.noreply.github.com> Date: Tue, 16 Sep 2025 09:46:46 +0200 Subject: [PATCH 1/2] ui/progress: Restore atomics in Counter We switched from atomics to a mutex in #3189 because of an alignment bug, but the new-style atomic types don't need manual alignment. --- internal/ui/progress/counter.go | 33 +++++++++------------------------ 1 file changed, 9 insertions(+), 24 deletions(-) diff --git a/internal/ui/progress/counter.go b/internal/ui/progress/counter.go index ac2e519a7..fbb5722e0 100644 --- a/internal/ui/progress/counter.go +++ b/internal/ui/progress/counter.go @@ -1,7 +1,7 @@ package progress import ( - "sync" + "sync/atomic" "time" ) @@ -17,17 +17,13 @@ type Func func(value uint64, total uint64, runtime time.Duration, final bool) // The Func is also called when SIGUSR1 (or SIGINFO, on BSD) is received. type Counter struct { Updater - - valueMutex sync.Mutex - value uint64 - max uint64 + value, max atomic.Uint64 } // NewCounter starts a new Counter. func NewCounter(interval time.Duration, total uint64, report Func) *Counter { - c := &Counter{ - max: total, - } + c := new(Counter) + c.max.Store(total) c.Updater = *NewUpdater(interval, func(runtime time.Duration, final bool) { v, maxV := c.Get() report(v, maxV, runtime, final) @@ -37,33 +33,22 @@ func NewCounter(interval time.Duration, total uint64, report Func) *Counter { // Add v to the Counter. This method is concurrency-safe. func (c *Counter) Add(v uint64) { - if c == nil { - return + if c != nil { + c.value.Add(v) } - - c.valueMutex.Lock() - c.value += v - c.valueMutex.Unlock() } // SetMax sets the maximum expected counter value. This method is concurrency-safe. func (c *Counter) SetMax(max uint64) { - if c == nil { - return + if c != nil { + c.max.Store(max) } - c.valueMutex.Lock() - c.max = max - c.valueMutex.Unlock() } // 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, max = c.value, c.max - c.valueMutex.Unlock() - - return v, max + return c.value.Load(), c.max.Load() } func (c *Counter) Done() { From a8f506ea4dfcbbca94271d8bb5d34a510378175b Mon Sep 17 00:00:00 2001 From: greatroar <61184462+greatroar@users.noreply.github.com> Date: Tue, 16 Sep 2025 09:56:33 +0200 Subject: [PATCH 2/2] ui/progress: Simplify Updater Removed a defer'd call that was a bit subtle. --- internal/ui/progress/updater.go | 10 +++------- internal/ui/progress/updater_test.go | 9 +++++++-- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/internal/ui/progress/updater.go b/internal/ui/progress/updater.go index 7fb6c8836..0f34b2d81 100644 --- a/internal/ui/progress/updater.go +++ b/internal/ui/progress/updater.go @@ -57,17 +57,13 @@ func (c *Updater) Done() { func (c *Updater) run() { defer close(c.stopped) - defer func() { - // Must be a func so that time.Since isn't called at defer time. - c.report(time.Since(c.start), true) - }() var tick <-chan time.Time if c.tick != nil { tick = c.tick.C } signalsCh := signals.GetProgressChannel() - for { + for final := false; !final; { var now time.Time select { @@ -76,9 +72,9 @@ func (c *Updater) run() { debug.Log("Signal received: %v\n", sig) now = time.Now() case <-c.stop: - return + final, now = true, time.Now() } - c.report(now.Sub(c.start), false) + c.report(now.Sub(c.start), final) } } diff --git a/internal/ui/progress/updater_test.go b/internal/ui/progress/updater_test.go index 45db019ba..3f7543697 100644 --- a/internal/ui/progress/updater_test.go +++ b/internal/ui/progress/updater_test.go @@ -9,13 +9,17 @@ import ( ) func TestUpdater(t *testing.T) { - finalSeen := false - var ncalls int + var ( + finalSeen = false + ncalls = 0 + dur time.Duration + ) report := func(d time.Duration, final bool) { if final { finalSeen = true } + dur = d ncalls++ } c := progress.NewUpdater(10*time.Millisecond, report) @@ -24,6 +28,7 @@ func TestUpdater(t *testing.T) { test.Assert(t, finalSeen, "final call did not happen") test.Assert(t, ncalls > 0, "no progress was reported") + test.Assert(t, dur > 0, "duration must be positive") } func TestUpdaterStopTwice(_ *testing.T) {