[BUG] v2.1.0 IntBetween still panic()'s... #169
Comments
|
Ack, I'm working to fix it. |
|
@sean- can you confirm which version of Go are you using? I'm not able to reproduce it with 1.22.0. |
|
@jaswdr I'm able to reproduce the error with I'm also able to reproduce the error with the latest version of faker EDIT: here's a smaller func to get the same result func main() {
f := faker.New()
for i := 0; i < 2; i++ {
go func() {
for {
f.IntBetween(0, 100)
}
}()
}
time.Sleep(time.Minute)
} |
|
I was Hacktoberfest but am not seeing how to resolve the issue reported, as there is a dilemma in this bug report. The goal of a seeded random generator is for reproducibility of results. But if you have concurrent processes, how would you ensure that the exact sequence of "random" numbers would repeat every time? Over a decade ago, the rand package got code comments because of the exact problem that seeded random number generators aren't safe for concurrent use. golang/go#3611
Faker is using rand.NewSource(time.Now().Unix()). This is clearly indicated in the rand package as not safe for concurrent use. If you don't mind losing the ability to have a seed, you could simply change func main() {
for i := 0; i < 2; i++ {
f := faker.New()
go func() {
for {
f.IntBetween(0, 100)
}
}()
}
time.Sleep(time.Minute)
}Another formulation of the benchmark, note that func BenchmarkIntBetweenThreadsafeParallel(b *testing.B) {
b.RunParallel(func(pb *testing.PB) {
f := New()
for pb.Next() {
f.IntBetween(1, 100)
}
})
}Assuming that the solution is to have one faker per concurrent thread, I might then suggest updating the seed to ensure that multiple faker.New created around the same time don't happen to have the same seed: // New returns a new instance of Faker instance with a random seed
func New() (f Faker) {
- seed := rand.NewSource(time.Now().Unix())
+ seed := rand.NewSource(rand.Int63())
f = NewWithSeed(seed)
return
}So finally, if you don't need seeded random numbers, you might simply implement your own threadsafe generator that can be used by multiple goroutines. // ThreadsafeGenerator is safe for concurrent use by multiple goroutines.
type ThreadsafeGenerator struct{}
// Intn returns, as an int, a non-negative pseudo-random number in [0,n).
// It panics if n <= 0.
func (m ThreadsafeGenerator) Intn(n int) int {
return rand.Intn(n)
}
// Int returns a non-negative pseudo-random int.
// It is safe for concurrent use by multiple goroutines.
func (m ThreadsafeGenerator) Int() int {
return rand.Int()
}
// Now that we use a threadsafe generator, we can use `f.IntBetween(1, 100)` concurrently.
func BenchmarkIntBetweenThreadsafeParallel(b *testing.B) {
f := New()
f.Generator = ThreadsafeGenerator{}
b.RunParallel(func(pb *testing.PB) {
for pb.Next() {
f.IntBetween(1, 100)
}
})
} |
Describe the bug
This looks like a continuation of #164
To Reproduce
This may still need to be entirely fixed. I can reproduce this about once every 50-100 runs or so. Using https://github.com/sean-/bench-go-histograms as the reproducer:
JFYI. The rate of this happening is now quite small, but it's not zero, either. I don't know if you want to open a new issue, or
The text was updated successfully, but these errors were encountered: