From f37d5f82272c99b6b60fd5730c85c2c07ef24697 Mon Sep 17 00:00:00 2001 From: Naveen Mahalingam Date: Wed, 29 May 2019 23:36:13 -0700 Subject: [PATCH] progress: handle race conditions; fixes #81 --- .travis.yml | 2 +- Makefile | 3 ++ cmd/demo-progress/demo.go | 2 +- progress/progress.go | 86 ++++++++++++++++++++++++++++----------- progress/render.go | 27 +++++++++--- progress/tracker.go | 25 ++++++++++++ 6 files changed, 113 insertions(+), 32 deletions(-) diff --git a/.travis.yml b/.travis.yml index e920465..6df98df 100644 --- a/.travis.yml +++ b/.travis.yml @@ -18,5 +18,5 @@ before_script: # CI Pipeline. script: - - make test bench + - make test test-race bench - $GOPATH/bin/goveralls -service=travis-pro -coverprofile=.coverprofile diff --git a/Makefile b/Makefile index 32e2b05..d6a052d 100644 --- a/Makefile +++ b/Makefile @@ -39,5 +39,8 @@ profile: test: fmt lint vet cyclo go test -cover -coverprofile=.coverprofile $(shell go list ./...) +test-race: + go run -race ./cmd/demo-progress/demo.go + vet: go vet $(shell go list ./...) diff --git a/cmd/demo-progress/demo.go b/cmd/demo-progress/demo.go index 3b51616..7d89c77 100644 --- a/cmd/demo-progress/demo.go +++ b/cmd/demo-progress/demo.go @@ -42,7 +42,7 @@ func trackSomething(pw progress.Writer, idx int64) { pw.AppendTracker(&tracker) - c := time.Tick(time.Millisecond * 250) + c := time.Tick(time.Millisecond * 100) for !tracker.IsDone() { select { case <-c: diff --git a/progress/progress.go b/progress/progress.go index 37d5bae..2b9747f 100644 --- a/progress/progress.go +++ b/progress/progress.go @@ -20,28 +20,32 @@ var ( // Progress helps track progress for one or more tasks. type Progress struct { - autoStop bool - done chan bool - lengthTracker int - lengthProgress int - outputWriter io.Writer - hideTime bool - hideTracker bool - hideValue bool - hidePercentage bool - messageWidth int - numTrackersExpected int64 - overallTracker *Tracker - renderInProgress bool - showOverallTracker bool - sortBy SortBy - style *Style - trackerPosition Position - trackersActive []*Tracker - trackersDone []*Tracker - trackersInQueue []*Tracker - trackersInQueueMutex sync.Mutex - updateFrequency time.Duration + autoStop bool + done chan bool + lengthTracker int + lengthProgress int + outputWriter io.Writer + hideTime bool + hideTracker bool + hideValue bool + hidePercentage bool + messageWidth int + numTrackersExpected int64 + overallTracker *Tracker + overallTrackerMutex sync.RWMutex + renderInProgress bool + renderInProgressMutex sync.RWMutex + showOverallTracker bool + sortBy SortBy + style *Style + trackerPosition Position + trackersActive []*Tracker + trackersActiveMutex sync.RWMutex + trackersDone []*Tracker + trackersDoneMutex sync.RWMutex + trackersInQueue []*Tracker + trackersInQueueMutex sync.RWMutex + updateFrequency time.Duration } // Position defines the position of the Tracker with respect to the Tracker's @@ -64,6 +68,7 @@ func (p *Progress) AppendTracker(t *Tracker) { t.Total = math.MaxInt64 } t.start() + p.overallTrackerMutex.Lock() if p.overallTracker == nil { p.overallTracker = &Tracker{Total: 1} if p.numTrackersExpected > 0 { @@ -73,10 +78,11 @@ func (p *Progress) AppendTracker(t *Tracker) { } p.trackersInQueueMutex.Lock() p.trackersInQueue = append(p.trackersInQueue, t) + p.trackersInQueueMutex.Unlock() if p.overallTracker.Total < int64(p.Length())*100 { p.overallTracker.Total = int64(p.Length()) * 100 } - p.trackersInQueueMutex.Unlock() + p.overallTrackerMutex.Unlock() } // AppendTrackers appends one or more Trackers for tracking. @@ -89,19 +95,51 @@ func (p *Progress) AppendTrackers(trackers []*Tracker) { // IsRenderInProgress returns true if a call to Render() was made, and is still // in progress and has not ended yet. func (p *Progress) IsRenderInProgress() bool { + p.renderInProgressMutex.RLock() + defer p.renderInProgressMutex.RUnlock() + return p.renderInProgress } // Length returns the number of Trackers tracked overall. func (p *Progress) Length() int { + p.trackersActiveMutex.RLock() + p.trackersDoneMutex.RLock() + p.trackersInQueueMutex.RLock() + defer p.trackersActiveMutex.RUnlock() + defer p.trackersDoneMutex.RUnlock() + defer p.trackersInQueueMutex.RUnlock() + return len(p.trackersInQueue) + len(p.trackersActive) + len(p.trackersDone) } // LengthActive returns the number of Trackers actively tracked (not done yet). func (p *Progress) LengthActive() int { + p.trackersActiveMutex.RLock() + p.trackersInQueueMutex.RLock() + defer p.trackersActiveMutex.RUnlock() + defer p.trackersInQueueMutex.RUnlock() + return len(p.trackersInQueue) + len(p.trackersActive) } +// LengthDone returns the number of Trackers that are done tracking. +func (p *Progress) LengthDone() int { + p.trackersDoneMutex.RLock() + defer p.trackersDoneMutex.RUnlock() + + return len(p.trackersDone) +} + +// LengthInQueue returns the number of Trackers in queue to be actively tracked +// (not tracking yet). +func (p *Progress) LengthInQueue() int { + p.trackersInQueueMutex.RLock() + defer p.trackersInQueueMutex.RUnlock() + + return len(p.trackersInQueue) +} + // SetAutoStop toggles the auto-stop functionality. Auto-stop set to true would // mean that the Render() function will automatically stop once all currently // active Trackers reach their final states. When set to false, the client code @@ -186,7 +224,7 @@ func (p *Progress) ShowValue(show bool) { // Stop stops the Render() logic that is in progress. func (p *Progress) Stop() { - if p.renderInProgress { + if p.IsRenderInProgress() { p.done <- true } } diff --git a/progress/render.go b/progress/render.go index ddf2cc5..d5f985b 100644 --- a/progress/render.go +++ b/progress/render.go @@ -11,19 +11,24 @@ import ( // Render renders the Progress tracker and handles all existing trackers and // those that are added dynamically while render is in progress. func (p *Progress) Render() { - if !p.renderInProgress { + if !p.IsRenderInProgress() { p.initForRender() c := time.Tick(p.updateFrequency) lastRenderLength := 0 - for p.renderInProgress = true; p.renderInProgress; { + p.renderInProgressMutex.Lock() + p.renderInProgress = true + p.renderInProgressMutex.Unlock() + for p.IsRenderInProgress() { select { case <-c: - if len(p.trackersInQueue) > 0 || len(p.trackersActive) > 0 { + if p.LengthActive() > 0 { lastRenderLength = p.renderTrackers(lastRenderLength) } case <-p.done: + p.renderInProgressMutex.Lock() p.renderInProgress = false + p.renderInProgressMutex.Unlock() } } } @@ -46,13 +51,17 @@ func (p *Progress) renderTrackers(lastRenderLength int) int { for _, tracker := range trackersDone { p.renderTracker(&out, tracker, renderHint{}) } + p.trackersDoneMutex.Lock() p.trackersDone = append(p.trackersDone, trackersDone...) + p.trackersDoneMutex.Unlock() // sort and render the active trackers for _, tracker := range trackersActive { p.renderTracker(&out, tracker, renderHint{}) } + p.trackersActiveMutex.Lock() p.trackersActive = trackersActive + p.trackersActiveMutex.Unlock() // render the overall tracker p.renderTracker(&out, p.overallTracker, renderHint{isOverallTracker: true}) @@ -61,7 +70,7 @@ func (p *Progress) renderTrackers(lastRenderLength int) int { p.outputWriter.Write([]byte(out.String())) // stop if auto stop is enabled and there are no more active trackers - if p.autoStop && len(p.trackersInQueue) == 0 && len(p.trackersActive) == 0 { + if p.autoStop && p.LengthActive() == 0 { p.done <- true } @@ -69,10 +78,12 @@ func (p *Progress) renderTrackers(lastRenderLength int) int { } func (p *Progress) consumeQueuedTrackers() { - if len(p.trackersInQueue) > 0 { + if p.LengthInQueue() > 0 { + p.trackersActiveMutex.Lock() p.trackersInQueueMutex.Lock() p.trackersActive = append(p.trackersActive, p.trackersInQueue...) p.trackersInQueue = make([]*Tracker, 0) + p.trackersActiveMutex.Unlock() p.trackersInQueueMutex.Unlock() } } @@ -84,6 +95,7 @@ func (p *Progress) extractDoneAndActiveTrackers() ([]*Tracker, []*Tracker) { // separate the active and done trackers var trackersActive, trackersDone []*Tracker var activeTrackersProgress int64 + p.trackersActiveMutex.RLock() for _, tracker := range p.trackersActive { if !tracker.IsDone() { trackersActive = append(trackersActive, tracker) @@ -92,11 +104,12 @@ func (p *Progress) extractDoneAndActiveTrackers() ([]*Tracker, []*Tracker) { trackersDone = append(trackersDone, tracker) } } + p.trackersActiveMutex.RUnlock() p.sortBy.Sort(trackersDone) p.sortBy.Sort(trackersActive) // calculate the overall tracker's progress value - p.overallTracker.value = int64(len(p.trackersDone)+len(trackersDone)) * 100 + p.overallTracker.value = int64(p.LengthDone()+len(trackersDone)) * 100 p.overallTracker.value += activeTrackersProgress if len(trackersActive) == 0 { p.overallTracker.MarkAsDone() @@ -105,10 +118,12 @@ func (p *Progress) extractDoneAndActiveTrackers() ([]*Tracker, []*Tracker) { } func (p *Progress) generateTrackerStr(t *Tracker, maxLen int) string { + t.mutex.Lock() pDotValue := float64(t.Total) / float64(maxLen) pFinishedDots := float64(t.value) / pDotValue pFinishedDotsFraction := pFinishedDots - float64(int(pFinishedDots)) pFinishedLen := int(math.Floor(pFinishedDots)) + t.mutex.Unlock() var pFinished, pInProgress, pUnfinished string if pFinishedLen > 0 { diff --git a/progress/tracker.go b/progress/tracker.go index ee58c99..595d167 100644 --- a/progress/tracker.go +++ b/progress/tracker.go @@ -1,6 +1,7 @@ package progress import ( + "sync" "time" ) @@ -21,6 +22,8 @@ type Tracker struct { Units Units done bool + mutex sync.RWMutex + mutexPrv sync.RWMutex timeStart time.Time timeStop time.Time value int64 @@ -29,6 +32,9 @@ type Tracker struct { // ETA returns the expected time of "arrival" or completion of this tracker. It // is an estimate and is not guaranteed. func (t *Tracker) ETA() time.Duration { + t.mutex.RLock() + defer t.mutex.RUnlock() + timeTaken := time.Since(t.timeStart) if t.ExpectedDuration > time.Duration(0) && t.ExpectedDuration > timeTaken { return t.ExpectedDuration - timeTaken @@ -43,29 +49,39 @@ func (t *Tracker) ETA() time.Duration { // Increment updates the current value of the task being tracked. func (t *Tracker) Increment(value int64) { + t.mutex.Lock() if !t.done { t.value += value if t.Total > 0 && t.value >= t.Total { t.stop() } } + t.mutex.Unlock() } // IsDone returns true if the tracker is done (value has reached the expected // Total set during initialization). func (t *Tracker) IsDone() bool { + t.mutex.RLock() + defer t.mutex.RUnlock() + return t.done } // MarkAsDone forces completion of the tracker by updating the current value as // the expected Total value. func (t *Tracker) MarkAsDone() { + t.mutex.Lock() t.Total = t.value t.stop() + defer t.mutex.Unlock() } // PercentDone returns the currently completed percentage value. func (t *Tracker) PercentDone() float64 { + t.mutex.RLock() + defer t.mutex.RUnlock() + if t.Total == 0 { return 0 } @@ -74,30 +90,39 @@ func (t *Tracker) PercentDone() float64 { // Reset resets the tracker to its initial state. func (t *Tracker) Reset() { + t.mutex.Lock() t.done = false t.timeStart = time.Time{} t.timeStop = time.Time{} t.value = 0 + t.mutex.Unlock() } // SetValue sets the value of the tracker and re-calculates if the tracker is // "done". func (t *Tracker) SetValue(value int64) { + t.mutex.Lock() t.done = false t.timeStop = time.Time{} t.value = 0 + t.mutex.Unlock() + t.Increment(value) } func (t *Tracker) start() { + t.mutexPrv.Lock() t.done = false t.timeStart = time.Now() + t.mutexPrv.Unlock() } 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() }