Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

progress: fix incorrect mutex usages #154

Merged
merged 2 commits into from
Apr 7, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions progress/progress.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package progress

import (
"io"
"math"
"os"
"sync"
"time"
Expand Down Expand Up @@ -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()
Expand Down
21 changes: 9 additions & 12 deletions progress/render.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 := ""
Expand Down Expand Up @@ -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 ")
Expand Down
24 changes: 17 additions & 7 deletions progress/tracker.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package progress

import (
"math"
"sync"
"time"
)
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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()
}