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>

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)
+	}
+}

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
+}