math/rand: document that math.Rand is not safe for concurrent use
gopherbot opened this issue · 7 comments
gopherbot commented
by gar3ts:
I have a project that as an aside calls rand.Intn() many times with different values. My project frequently causes rng.Int63() to panic due to an out of range index: math/rand.(*rngSource).Int63(0xf84006a400, 0xf8412cc2a0, 0xf840050ab0, 0xf84006a400) C:/Go/src/pkg/math/rand/rng.go:243 +0x7e math/rand.(*Rand).Int63(0xf84003d5e0, 0x200027bb78, 0x42e0eb, 0xf84003d5e0) C:/Go/src/pkg/math/rand/rand.go:37 +0x46 math/rand.(*Rand).Int31(0xf84003d5e0, 0x414b1b, 0x444369, 0x27bb60) C:/Go/src/pkg/math/rand/rand.go:43 +0x28 math/rand.(*Rand).Int31n(0xf84003d5e0, 0x3, 0x27bb60, 0x43c9c1, 0x20, ...) C:/Go/src/pkg/math/rand/rand.go:72 +0x79 math/rand.(*Rand).Intn(0xf84003d5e0, 0xf800000003, 0xf84104a5c8, 0xf841e9b160, 0xf84244e300, ...) C:/Go/src/pkg/math/rand/rand.go:86 +0x70 Which compiler are you using (5g, 6g, 8g, gccgo)? go run Which operating system are you using? Windows7 Which version are you using? (run 'go version') 1.0.1 but 1.0 had the same problem Please provide any additional information below. it only happens when I'm using multiple procs, i.e.: runtime.GOMAXPROCS(2) The calls to rand are made from various of 1000's of goroutine instances. I instrumented Int63() to capture useful values in a recover() then downloaded MinGW and compiled Go for amd64. Here's some of the captured state: seed: 1336714659800372000 count: 1003446 // number of calls to Int63() rng.feed: 330 rng.tap: 1212 another: seed: 1336715957940621400 count: 726597 rng.feed: 333 rng.tap: 1212 another: seed: 1336716097220587700 count: 478936 rng.feed: 327 rng.tap: 1212 note that len(rng.vec) is 607 rng.fee and rng.tap are used as indexes into that array so for the above examples it is clear that rng.tap is the problem. What is unclear is how it manages to get out of range. The following are captures from an index out of range panic but without the values being out of range somehow: rng.seed: 1336716288342519300 count: 390314 rng.feed: 337 rng.tap: 604 and: rng.seed: 1336717800504010000 count: 143252 rng.feed: 334 rng.tap: 606 This looks like a thread safety problem but http://golang.org/pkg/math/rand/ doesn't mention it. Also, I don't get a panic in my project when I substitute a cryptographic random number generator.
ianlancetaylor commented
robpike commented
rsc commented
In general the rule is this: top-level functions like strings.Split or fmt.Printf or rand.Int63 may be called from any goroutine at any time (otherwise programming with them would be too restrictive), but objects you create (like a new bytes.Buffer or rand.Rand) must only be used by one goroutine at a time unless otherwise noted (like net.Conn's documentation does). There are not enough stack frames to say for sure, but it sounds like you are calling Int63 on your own allocated rand.Rand from multiple goroutines. That is not promised to work, and it turns out does not. If you are actually calling the top-level function rand.Int63 and it is crashing, then that's a bug on our part and we should investigate further. Please let us know which it is. Thanks.
Status changed to WaitingForReply.
gopherbot commented
I'm calling rand.Intn(int) from a rand that I created. Is the best practice to use a mutex to control access to the rand I create? Additionally, couldn't the code in rng.go be made goroutine-safe (preventing tap and feed from jumping off the end of the vector) by doing the decrement and range fix against a local variable? tap := rng.tap -1 if tap < 0 { tap += _LEN } rng.tap = tap feed := rng.feed -1 if feed < 0 { feed += _LEN } rng.feed = feed Thanks.
rsc commented
: I'm calling rand.Intn(int) from a rand that I created. Is the best practice : to use a mutex to control access to the rand I create? Yes, using a mutex is the right solution, or call the package's top-level Intn function. : Additionally, couldn't the code in rng.go be made goroutine-safe (preventing : tap and feed from jumping off the end of the vector) by doing the decrement : and range fix against a local variable? The rewrite would probably work, but you still have the problem that simultaneous calls to Intn are returning the same value. Also, an optimizing compiler would be allowed to rewrite your new code into the old code. Russ
gopherbot commented
OK thanks. I'm creating different rands in order to reduce contention for the top-level rand. Please consider a revision along the lines I suggested. I'd think different rands getting the same value occasionally would be preferable to an out-of-range exception - at least where the compiler doesn't optimize out the temp variables.
rsc commented