Progress keeps printing after .Stop()

[mgurov]

Simple demonstration:

func ExampleStoppingPrintout() {
    progress := uiprogress.New()
    progress.RefreshInterval = time.Millisecond * 10
    progress.Start()
    bar := progress.AddBar(1)
    bar.Incr()
    time.Sleep(time.Millisecond * 15)
        //workaround
    //progress.Bars = nil
    progress.Stop()
    time.Sleep(1 * time.Second)
    // Output: [====================================================================]
}

Expected: bar once printed, as it is stopped after one interval.
Actual: The bar is printed every 10 milliseconds.

[mgurov]

The cause of this issue is that the .Listen() doesn't make copy of the Progress.closeChan, and misses the moment when it is replaced by nil at .Stop().

[mgurov]

A simple, albeit imperfect workaround is to remove the bars from the progress:

progress.Bars = nil

it is still not perfect as there will be one more printout when the .Listen() awakes after time.Sleep(p.RefreshInterval).

[henvic]

I have the same issue.

[mgurov]

Simplest solution would be to make a copy of the stop chan and use time.Ticker instead of time.Sleep:

func (p *Progress) Listen() {
    p.lw.Out = p.Out
    stopChan := p.stopChan
    ticker := time.NewTicker(p.RefreshInterval)
    for {
        select {
        case <-stopChan:
            ticker.Stop()
            return
        case <- ticker.C:
            p.mtx.RLock()
            for _, bar := range p.Bars {
                fmt.Fprintln(p.lw, bar.String())
            }
            p.lw.Flush()
            p.mtx.RUnlock()
        }
    }
}

a draft can be found at my branch where I try to fight racing conditions of the Start/Stop for the travis to be happy with my new tests.

Even better solution IMHO would be to avoid swapping Progress.closeChan and instead make it not restartable. But I am not sure how much backwards breaking would such change be for the clients of the library.

[henvic]

@mgurov,
are you writing a pull request for this?

I have tried your fix and here are my findings:

1. works most of the times with multi (2) progress bars, but sometimes the progress bars are printed more than once (don't know if this is related, or an entirely new issue) and the output just after the Stop might not appear when this happens
2. almost always works with a single progress bars, but sometimes the output doesn't appear (as in: is cleared)
3. if I add a great delay, say, time.Sleep(300 * time.Millisecond) after the stop and before the printing to stdout, it always works.

[mgurov]

@henvic thank you for the feedback.

(2. and (3. might be related to #18 - the bar doesn't print out properly. That hints that might fix might need to be extended correspondingly. For example doing that flush on stop.

(1. Not clear to me. I guess with the current master the behavior is not much better?

I won't probably be working very active on a PR before the weekend. Feel free to jump in.

[henvic]

Regarding 2: with the current master, the behavior is worse (Flushing before stop should fix it).

I don't know if I am going to be able to work on a PR, but I'll let you know if I have any news.

[revett]

This fixed it for me! Thanks 👍

progress.Bars = nil

[henvic]

@revett, it should solve for 99.9% of the cases, though you will still find the issue sometimes.