add mutex to progress for prevent race condition #81
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…r prevent race condition
|
That is a lot of additional mutex usage. Can you share a sample code where you see the race condition? Maybe there is a simpler solution that we can work out. |
|
Hi @jedib0t, there is the data race warning message output during I'm created at least 3 gorouting to process the data with its progressbar, and it also might caused a hanging by rapidly calling and here is a sample code I reproduced the same situation and log code package main
import (
"math/rand"
"os"
"os/signal"
"sync"
"syscall"
"time"
"github.com/jedib0t/go-pretty/progress"
)
func getProgress() bool {
syncProgress.RLock()
progress := (*progressWriter).IsRenderInProgress()
syncProgress.RUnlock()
return progress
}
func getLength() int {
syncProgress.RLock()
length := (*progressWriter).LengthActive()
syncProgress.RUnlock()
return length
}
func NewProgress() *progress.Writer {
syncProgress.Lock()
if progressRendered == false {
go (*progressWriter).Render()
progressRendered = true
}
syncProgress.Unlock()
return progressWriter
}
func EndProgress() {
for getProgress() {
// for manual-stop mode, stop when there are no more active trackers
if getLength() == 0 {
(*progressWriter).Stop()
syncProgress.Lock()
progressRendered = false
syncProgress.Unlock()
}
time.Sleep(time.Millisecond * 100)
}
}
var progressWriter *progress.Writer
var syncProgress = sync.RWMutex{}
var progressRendered = false
func init() {
pw := progress.NewWriter()
pw.SetAutoStop(false)
pw.SetSortBy(progress.SortByNone)
pw.SetStyle(progress.StyleDefault)
pw.SetTrackerPosition(progress.PositionRight)
pw.SetUpdateFrequency(time.Millisecond * 100)
pw.Style().Colors = progress.StyleColorsExample
pw.Style().Options.PercentFormat = "%4.1f%%"
progressWriter = &pw
}
func sample1() {
var wg sync.WaitGroup
pw := NewProgress()
tracker := progress.Tracker{
Message: "sample1",
Total: 40,
Units: progress.UnitsDefault,
}
(*pw).AppendTracker(&tracker)
for i := 1; i <= 40; i++ {
wg.Add(1)
go func() {
defer tracker.Increment(1)
defer wg.Done()
time.Sleep(time.Duration(rand.Float64()*100) * time.Millisecond)
}()
}
wg.Wait()
EndProgress()
}
func sample2() {
var wg sync.WaitGroup
pw := NewProgress()
tracker := progress.Tracker{
Message: "sample2",
Total: 20,
Units: progress.UnitsDefault,
}
(*pw).AppendTracker(&tracker)
for i := 1; i <= 20; i++ {
wg.Add(1)
go func() {
defer tracker.Increment(1)
defer wg.Done()
time.Sleep(time.Duration(rand.Float64()*200) * time.Millisecond)
}()
}
wg.Wait()
EndProgress()
}
func sample3() {
var wg sync.WaitGroup
pw := NewProgress()
tracker := progress.Tracker{
Message: "sample3",
Total: 10,
Units: progress.UnitsDefault,
}
(*pw).AppendTracker(&tracker)
for i := 1; i <= 10; i++ {
wg.Add(1)
go func() {
defer tracker.Increment(1)
defer wg.Done()
time.Sleep(time.Duration(rand.Float64()*400) * time.Millisecond)
}()
}
wg.Wait()
EndProgress()
}
var swg sync.WaitGroup
func main() {
go sample1()
go sample2()
go sample3()
// Handle SIGINT and SIGTERM
sig := make(chan os.Signal)
signal.Notify(sig, syscall.SIGHUP, syscall.SIGINT, syscall.SIGTERM)
// Wait signal
swg.Add(1)
go func() {
select {
case s := <-sig:
if s != nil {
switch s {
case syscall.SIGINT, syscall.SIGTERM:
swg.Done()
}
}
}
}()
swg.Wait()
signal.Stop(sig)
close(sig)
}log |
|
after my latest update of #62aac18, it fixed the data race with same sample code w/o any data race warnings like this below |
|
Weird. Your sample code doesn't seem to throw any race conditions on my machine. Can you share your system details? OS? Golang version? |
|
Here is
|
|
Cool looking prompt! :) I'm running golang 1.10 and am not able to reproduce the bug. I'll install 1.12.5 and try it. |
|
I'm using |
|
tested with |
|
Hey. I tried reproducing it in go1.10.4 in WSL Ubuntu-18.04.2 and also bare Ubuntu-18.04.2 (both x86_64) and wasn't able to. I'll try more versions this week and let you know if I can reproduce it and find the root-cause. BTW, I see that the sample code you pasted is calling Render() three times (once in each goroutine via Ideally, this is how you do progress tracking using this package:
Sample usage is at |
|
Just noticed the |
|
Hey @Haraguroicha can you please rebase your branch? I'll merge your changes once done. |
jedib0t
added a commit
that referenced
this pull request
May 30, 2019
jedib0t
added a commit
that referenced
this pull request
May 30, 2019
jedib0t
added a commit
that referenced
this pull request
May 30, 2019
jedib0t
added a commit
that referenced
this pull request
May 30, 2019
jedib0t
added a commit
that referenced
this pull request
May 30, 2019
|
Hey @Haraguroicha --- I've used your code and added some fixes on top with #83 and merged it into master. Will cut a new tag soon that you can use with all these fixes. I really appreciate you taking the time to debug and fix the unsafe accesses in the code. Thanks a ton! |
|
Hey @Haraguroicha --- please use tag |
|
Hi @jedib0t, in my sample code of func NewProgress() *progress.Writer {
syncProgress.Lock()
if progressRendered == false {
go (*progressWriter).Render()
progressRendered = true
}
syncProgress.Unlock()
return progressWriter
} |
|
Oops, I saw you have merged as tag |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
it might not the best way to fix the race condition, but it fixed the overwritten in race condition and stable in thread safe state when many goroutine called progress render.