math/rand: fix raciness in Rand.Read

There are no synchronization points protecting the readVal and readPos
variables. This leads to a race when Read is called concurrently.
Fix this by adding methods to lockedSource, which is the case where
a race matters.

Fixes #16308.

Change-Id: Ic028909955700906b2d71e5c37c02da21b0f4ad9
Reviewed-on: https://go-review.googlesource.com/24852
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>

diff --git a/src/math/rand/race_test.go b/src/math/rand/race_test.go
new file mode 100644 (file)
index 0000000..48f6c29
--- /dev/null
+++ b/src/math/rand/race_test.go
@@ -0,0 +1,47 @@
+// Copyright 2016 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package rand
+
+import (
+       "sync"
+       "testing"
+)
+
+// TestConcurrent exercises the rand API concurrently, triggering situations
+// where the race detector is likely to detect issues.
+func TestConcurrent(t *testing.T) {
+       const (
+               numRoutines = 10
+               numCycles   = 10
+       )
+       var wg sync.WaitGroup
+       defer wg.Wait()
+       wg.Add(numRoutines)
+       for i := 0; i < numRoutines; i++ {
+               go func(i int) {
+                       defer wg.Done()
+                       buf := make([]byte, 997)
+                       for j := 0; j < numCycles; j++ {
+                               var seed int64
+                               seed += int64(ExpFloat64())
+                               seed += int64(Float32())
+                               seed += int64(Float64())
+                               seed += int64(Intn(Int()))
+                               seed += int64(Int31n(Int31()))
+                               seed += int64(Int63n(Int63()))
+                               seed += int64(NormFloat64())
+                               seed += int64(Uint32())
+                               for _, p := range Perm(10) {
+                                       seed += int64(p)
+                               }
+                               Read(buf)
+                               for _, b := range buf {
+                                       seed += int64(b)
+                               }
+                               Seed(int64(i*j) * seed)
+                       }
+               }(i)
+       }
+}

diff --git a/src/math/rand/rand.go b/src/math/rand/rand.go
index 8f31b0ea9d3f5867329a4db3a5f4f1022e49f7d8..dd8d43cca1c18d81fd57419d802aa7e29c96d790 100644 (file)
--- a/src/math/rand/rand.go
+++ b/src/math/rand/rand.go
@@ -49,7 +49,13 @@ type Rand struct {
 func New(src Source) *Rand { return &Rand{src: src} }
 
 // Seed uses the provided seed value to initialize the generator to a deterministic state.
+// Seed should not be called concurrently with any other Rand method.
 func (r *Rand) Seed(seed int64) {
+       if lk, ok := r.src.(*lockedSource); ok {
+               lk.seedPos(seed, &r.readPos)
+               return
+       }
+
        r.src.Seed(seed)
        r.readPos = 0
 }
@@ -172,20 +178,28 @@ func (r *Rand) Perm(n int) []int {
 
 // Read generates len(p) random bytes and writes them into p. It
 // always returns len(p) and a nil error.
+// Read should not be called concurrently with any other Rand method.
 func (r *Rand) Read(p []byte) (n int, err error) {
-       pos := r.readPos
-       val := r.readVal
+       if lk, ok := r.src.(*lockedSource); ok {
+               return lk.read(p, &r.readVal, &r.readPos)
+       }
+       return read(p, r.Int63, &r.readVal, &r.readPos)
+}
+
+func read(p []byte, int63 func() int64, readVal *int64, readPos *int8) (n int, err error) {
+       pos := *readPos
+       val := *readVal
        for n = 0; n < len(p); n++ {
                if pos == 0 {
-                       val = r.Int63()
+                       val = int63()
                        pos = 7
                }
                p[n] = byte(val)
                val >>= 8
                pos--
        }
-       r.readPos = pos
-       r.readVal = val
+       *readPos = pos
+       *readVal = val
        return
 }
 
@@ -199,6 +213,7 @@ var globalRand = New(&lockedSource{src: NewSource(1)})
 // deterministic state. If Seed is not called, the generator behaves as
 // if seeded by Seed(1). Seed values that have the same remainder when
 // divided by 2^31-1 generate the same pseudo-random sequence.
+// Seed, unlike the Rand.Seed method, is safe for concurrent use.
 func Seed(seed int64) { globalRand.Seed(seed) }
 
 // Int63 returns a non-negative pseudo-random 63-bit integer as an int64
@@ -245,6 +260,7 @@ func Perm(n int) []int { return globalRand.Perm(n) }
 
 // Read generates len(p) random bytes from the default Source and
 // writes them into p. It always returns len(p) and a nil error.
+// Read, unlike the Rand.Read method, is safe for concurrent use.
 func Read(p []byte) (n int, err error) { return globalRand.Read(p) }
 
 // NormFloat64 returns a normally distributed float64 in the range
@@ -285,3 +301,19 @@ func (r *lockedSource) Seed(seed int64) {
        r.src.Seed(seed)
        r.lk.Unlock()
 }
+
+// seedPos implements Seed for a lockedSource without a race condiiton.
+func (r *lockedSource) seedPos(seed int64, readPos *int8) {
+       r.lk.Lock()
+       r.src.Seed(seed)
+       *readPos = 0
+       r.lk.Unlock()
+}
+
+// read implements Read for a lockedSource without a race condition.
+func (r *lockedSource) read(p []byte, readVal *int64, readPos *int8) (n int, err error) {
+       r.lk.Lock()
+       n, err = read(p, r.src.Int63, readVal, readPos)
+       r.lk.Unlock()
+       return
+}