From 800ef38cf6ce1bbcbafc9840b4f081123ef50277 Mon Sep 17 00:00:00 2001 From: Mykola Gurov Date: Wed, 11 May 2016 22:06:57 +0200 Subject: [PATCH 1/7] Using time.Ticker for running stoppable listener. Fix edge case where flush happens after the listener is stopped. --- progress.go | 44 +++++++++++++++++++++++++------------------- progress_test.go | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 19 deletions(-) create mode 100644 progress_test.go diff --git a/progress.go b/progress.go index a93838d..5bf16f5 100644 --- a/progress.go +++ b/progress.go @@ -33,9 +33,9 @@ type Progress struct { // RefreshInterval in the time duration to wait for refreshing the output RefreshInterval time.Duration - lw *uilive.Writer - stopChan chan struct{} - mtx *sync.RWMutex + lw *uilive.Writer + ticker *time.Ticker + mtx *sync.RWMutex } // New returns a new progress bar with defaults @@ -46,9 +46,8 @@ func New() *Progress { Bars: make([]*Bar, 0), RefreshInterval: RefreshInterval, - lw: uilive.New(), - stopChan: make(chan struct{}), - mtx: &sync.RWMutex{}, + lw: uilive.New(), + mtx: &sync.RWMutex{}, } } @@ -86,34 +85,41 @@ func (p *Progress) AddBar(total int) *Bar { // Listen listens for updates and renders the progress bars func (p *Progress) Listen() { p.lw.Out = p.Out - for { - select { - case <-p.stopChan: - return - default: - time.Sleep(p.RefreshInterval) + + go func() { + for _ = range p.ticker.C { p.mtx.RLock() - for _, bar := range p.Bars { - fmt.Fprintln(p.lw, bar.String()) - } + p.print() p.lw.Flush() p.mtx.RUnlock() } + }() +} + +func (p *Progress) print() { + for _, bar := range p.Bars { + fmt.Fprintln(p.lw, bar.String()) } } // Start starts the rendering the progress of progress bars. It listens for updates using `bar.Set(n)` and new bars when added using `AddBar` func (p *Progress) Start() { - if p.stopChan == nil { - p.stopChan = make(chan struct{}) + p.mtx.Lock() + if p.ticker == nil { + p.ticker = time.NewTicker(RefreshInterval) } + p.mtx.Unlock() + go p.Listen() } // Stop stops listening func (p *Progress) Stop() { - close(p.stopChan) - p.stopChan = nil + p.mtx.Lock() + p.ticker.Stop() + p.print() + p.lw.Flush() + p.mtx.Unlock() } // Bypass returns a writer which allows non-buffered data to be written to the underlying output diff --git a/progress_test.go b/progress_test.go new file mode 100644 index 0000000..788b710 --- /dev/null +++ b/progress_test.go @@ -0,0 +1,46 @@ +package uiprogress + +import ( + "bytes" + "fmt" + "strings" + "sync" + "testing" + "time" +) + +func TestStoppingPrintout(t *testing.T) { + progress := New() + progress.RefreshInterval = time.Millisecond * 10 + var buffer = &bytes.Buffer{} + progress.Out = buffer + bar := progress.AddBar(100) + progress.Start() + + var wg sync.WaitGroup + + wg.Add(1) + + go func() { + for i := 0; i <= 80; i = i + 10 { + bar.Set(i) + time.Sleep(time.Millisecond * 5) + } + + wg.Done() + }() + + wg.Wait() + + progress.Stop() + fmt.Fprintf(buffer, "foo") + + // give some time to see if any changes are made after Stop() is called + time.Sleep(time.Millisecond * 20) + + var wantSuffix = "[======================================================>-------------]\nfoo" + + if !strings.HasSuffix(buffer.String(), wantSuffix) { + t.Errorf("Content that should be printed after stop not appearing on buffer.") + } +} From f0722e107b23fa221cb9bbe0ec411728ffc1adc2 Mon Sep 17 00:00:00 2001 From: Henrique Vicente Date: Thu, 19 May 2016 03:41:54 -0300 Subject: [PATCH 2/7] Allow the progress to be restarted. --- progress.go | 1 + 1 file changed, 1 insertion(+) diff --git a/progress.go b/progress.go index 5bf16f5..09bef1a 100644 --- a/progress.go +++ b/progress.go @@ -117,6 +117,7 @@ func (p *Progress) Start() { func (p *Progress) Stop() { p.mtx.Lock() p.ticker.Stop() + p.ticker = nil p.print() p.lw.Flush() p.mtx.Unlock() From 528664d60adadcf4fd17b1129b6400d5947c7460 Mon Sep 17 00:00:00 2001 From: Henrique Vicente Date: Thu, 19 May 2016 04:13:40 -0300 Subject: [PATCH 3/7] Closing ticker.Stop() to avoid leak and allow to restart the listener. --- progress.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/progress.go b/progress.go index 09bef1a..a1d3383 100644 --- a/progress.go +++ b/progress.go @@ -35,6 +35,7 @@ type Progress struct { lw *uilive.Writer ticker *time.Ticker + tdone chan bool mtx *sync.RWMutex } @@ -86,14 +87,19 @@ func (p *Progress) AddBar(total int) *Bar { func (p *Progress) Listen() { p.lw.Out = p.Out - go func() { - for _ = range p.ticker.C { + for { + select { + case <-p.ticker.C: p.mtx.RLock() p.print() p.lw.Flush() p.mtx.RUnlock() + case <-p.tdone: + p.ticker.Stop() + p.ticker = nil + return } - }() + } } func (p *Progress) print() { @@ -107,6 +113,7 @@ func (p *Progress) Start() { p.mtx.Lock() if p.ticker == nil { p.ticker = time.NewTicker(RefreshInterval) + p.tdone = make(chan bool, 1) } p.mtx.Unlock() @@ -116,8 +123,7 @@ func (p *Progress) Start() { // Stop stops listening func (p *Progress) Stop() { p.mtx.Lock() - p.ticker.Stop() - p.ticker = nil + close(p.tdone) p.print() p.lw.Flush() p.mtx.Unlock() From a4d347ab9eb02b37e78f61a4e0d263b99ab71c1c Mon Sep 17 00:00:00 2001 From: Henrique Vicente Date: Tue, 24 May 2016 16:09:10 -0300 Subject: [PATCH 4/7] Test if ticker still exist before printing and flushing progress bar on tick. --- progress.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/progress.go b/progress.go index a1d3383..b83b4f0 100644 --- a/progress.go +++ b/progress.go @@ -91,8 +91,12 @@ func (p *Progress) Listen() { select { case <-p.ticker.C: p.mtx.RLock() - p.print() - p.lw.Flush() + + if p.ticker != nil { + p.print() + p.lw.Flush() + } + p.mtx.RUnlock() case <-p.tdone: p.ticker.Stop() From a54c7b78660b5ed42469279ca6d94a5f3a5e9056 Mon Sep 17 00:00:00 2001 From: Henrique Vicente Date: Tue, 24 May 2016 16:09:37 -0300 Subject: [PATCH 5/7] Removing wrongly commited time.Sleep call on the test. --- progress_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/progress_test.go b/progress_test.go index 788b710..d6a1b42 100644 --- a/progress_test.go +++ b/progress_test.go @@ -35,9 +35,6 @@ func TestStoppingPrintout(t *testing.T) { progress.Stop() fmt.Fprintf(buffer, "foo") - // give some time to see if any changes are made after Stop() is called - time.Sleep(time.Millisecond * 20) - var wantSuffix = "[======================================================>-------------]\nfoo" if !strings.HasSuffix(buffer.String(), wantSuffix) { From e3087cc1cf75a5ef693bc7426dcd70ee16754417 Mon Sep 17 00:00:00 2001 From: Henrique Vicente Date: Tue, 24 May 2016 18:45:47 -0300 Subject: [PATCH 6/7] Verify if p.ticker exists, before trying to stop it. --- progress.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/progress.go b/progress.go index b83b4f0..5d87135 100644 --- a/progress.go +++ b/progress.go @@ -99,9 +99,11 @@ func (p *Progress) Listen() { p.mtx.RUnlock() case <-p.tdone: - p.ticker.Stop() - p.ticker = nil - return + if p.ticker != nil { + p.ticker.Stop() + p.ticker = nil + return + } } } } From e3db2fd8ec4bb4d8f339a25a1b22204bab8f2fa5 Mon Sep 17 00:00:00 2001 From: Henrique Vicente Date: Wed, 25 May 2016 23:20:13 -0300 Subject: [PATCH 7/7] Adding tickChain variable to avoid nil dereference error. --- progress.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/progress.go b/progress.go index 5d87135..759a1a1 100644 --- a/progress.go +++ b/progress.go @@ -85,11 +85,12 @@ func (p *Progress) AddBar(total int) *Bar { // Listen listens for updates and renders the progress bars func (p *Progress) Listen() { + var tickChan = p.ticker.C p.lw.Out = p.Out for { select { - case <-p.ticker.C: + case <-tickChan: p.mtx.RLock() if p.ticker != nil {