progress: fix incorrect mutex usages (#154)

progress/progress.go

@@ -2,7 +2,6 @@ package progress
 
 import (
 	"io"
-	"math"
 	"os"
 	"sync"
 	"time"
@@ -65,11 +64,6 @@ const (
 // to a queue, which gets picked up by the Render logic in the next rendering
 // cycle.
 func (p *Progress) AppendTracker(t *Tracker) {
-	t.mutex.Lock()
-	if t.Total < 0 {
-		t.Total = math.MaxInt64
-	}
-	t.mutex.Unlock()
 	t.start()
 
 	p.overallTrackerMutex.Lock()

progress/render.go

@@ -123,24 +123,23 @@ func (p *Progress) extractDoneAndActiveTrackers() ([]*Tracker, []*Tracker) {
 }
 
 func (p *Progress) generateTrackerStr(t *Tracker, maxLen int, hint renderHint) string {
-	if !hint.isOverallTracker && (t.Total == 0 || t.value > t.Total) {
-		return p.generateTrackerStrIndeterminate(t, maxLen)
+	value, total := t.valueAndTotal()
+	if !hint.isOverallTracker && (total == 0 || value > total) {
+		return p.generateTrackerStrIndeterminate(maxLen)
 	}
-	return p.generateTrackerStrDeterminate(t, maxLen)
+	return p.generateTrackerStrDeterminate(value, total, maxLen)
 }
 
 // generateTrackerStrDeterminate generates the tracker string for the case where
 // the Total value is known, and the progress percentage can be calculated.
-func (p *Progress) generateTrackerStrDeterminate(t *Tracker, maxLen int) string {
-	t.mutex.Lock()
+func (p *Progress) generateTrackerStrDeterminate(value int64, total int64, maxLen int) string {
 	pFinishedDots, pFinishedDotsFraction := 0.0, 0.0
-	pDotValue := float64(t.Total) / float64(maxLen)
+	pDotValue := float64(total) / float64(maxLen)
 	if pDotValue > 0 {
-		pFinishedDots = float64(t.value) / pDotValue
+		pFinishedDots = float64(value) / pDotValue
 		pFinishedDotsFraction = pFinishedDots - float64(int(pFinishedDots))
 	}
 	pFinishedLen := int(math.Floor(pFinishedDots))
-	t.mutex.Unlock()
 
 	var pFinished, pInProgress, pUnfinished string
 	if pFinishedLen > 0 {
@@ -168,7 +167,7 @@ func (p *Progress) generateTrackerStrDeterminate(t *Tracker, maxLen int) string
 
 // generateTrackerStrDeterminate generates the tracker string for the case where
 // the Total value is unknown, and the progress percentage cannot be calculated.
-func (p *Progress) generateTrackerStrIndeterminate(t *Tracker, maxLen int) string {
+func (p *Progress) generateTrackerStrIndeterminate(maxLen int) string {
 	indicator := p.style.Chars.Indeterminate(maxLen)
 
 	pUnfinished := ""
@@ -287,9 +286,7 @@ func (p *Progress) renderTrackerStats(out *strings.Builder, t *Tracker, hint ren
 		var outStats strings.Builder
 		outStats.WriteString(" [")
 		if !hint.hideValue {
-			t.mutex.Lock()
-			outStats.WriteString(p.style.Colors.Value.Sprint(t.Units.Sprint(t.value)))
-			t.mutex.Unlock()
+			outStats.WriteString(p.style.Colors.Value.Sprint(t.Units.Sprint(t.Value())))
 		}
 		if !hint.hideValue && !hint.hideTime {
 			outStats.WriteString(" in ")

progress/tracker.go

@@ -1,6 +1,7 @@
 package progress
 
 import (
+	"math"
 	"sync"
 	"time"
 )
@@ -24,7 +25,6 @@ type Tracker struct {
 	done      bool
 	err       bool
 	mutex     sync.RWMutex
-	mutexPrv  sync.RWMutex
 	timeStart time.Time
 	timeStop  time.Time
 	value     int64
@@ -141,11 +141,19 @@ func (t *Tracker) SetValue(value int64) {
 
 // Value returns the current value of the tracker.
 func (t *Tracker) Value() int64 {
-	t.mutex.Lock()
-	defer t.mutex.Unlock()
+	t.mutex.RLock()
+	defer t.mutex.RUnlock()
 	return t.value
 }
 
+func (t *Tracker) valueAndTotal() (int64, int64) {
+	t.mutex.RLock()
+	value := t.value
+	total := t.Total
+	t.mutex.RUnlock()
+	return value, total
+}
+
 func (t *Tracker) incrementWithoutLock(value int64) {
 	if !t.done {
 		t.value += value
@@ -156,19 +164,21 @@ func (t *Tracker) incrementWithoutLock(value int64) {
 }
 
 func (t *Tracker) start() {
-	t.mutexPrv.Lock()
+	t.mutex.Lock()
+	if t.Total < 0 {
+		t.Total = math.MaxInt64
+	}
 	t.done = false
 	t.err = false
 	t.timeStart = time.Now()
-	t.mutexPrv.Unlock()
+	t.mutex.Unlock()
 }
 
+// this must be called with the mutex held with a write lock
 func (t *Tracker) stop() {
-	t.mutexPrv.Lock()
 	t.done = true
 	t.timeStop = time.Now()
 	if t.value > t.Total {
 		t.Total = t.value
 	}
-	t.mutexPrv.Unlock()
 }