Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

math/rand: use of Read is racy #16308

Closed
yonderblue opened this issue Jul 9, 2016 · 4 comments
Closed

math/rand: use of Read is racy #16308

yonderblue opened this issue Jul 9, 2016 · 4 comments
Milestone

Comments

@yonderblue
Copy link

@yonderblue yonderblue commented Jul 9, 2016

Please answer these questions before submitting your issue. Thanks!

  1. What version of Go are you using (go version)?
    1.7rc1
  2. What operating system and processor architecture are you using (go env)?
    linux amd64
  3. What did you do?
    Ran a large concurrent unit test.
  4. What did you expect to see?
    Bunnies
  5. What did you see instead?
WARNING: DATA RACE
Write at 0x00c420098098 by goroutine 65:
  math/rand.(*Rand).Seed()
      /usr/local/go/src/math/rand/rand.go:54 +0x6d
  math/rand.Seed()
      /usr/local/go/src/math/rand/rand.go:202 +0x54
  XXX/XXX_test.TestXXX.func2()
      /go/src/XXX/XXX_test.go:359 +0x163

Previous write at 0x00c420098098 by goroutine 66:
  math/rand.(*Rand).Seed()
      /usr/local/go/src/math/rand/rand.go:54 +0x6d
  math/rand.Seed()
      /usr/local/go/src/math/rand/rand.go:202 +0x54
  XXX/XXX_test.TestXXX.func2()
      /go/src/XXX/XXX_test.go:359 +0x163

Goroutine 65 (running) created at:
  XXX/XXX_test.TestXXX()
      /go/src/XXX/XXX_test.go:389 +0x8c2
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:610 +0xc9

Goroutine 66 (running) created at:
  XXX/XXX_test.TestXXX()
      /go/src/XXX/XXX_test.go:389 +0x8c2
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:610 +0xc9
==================
==================
WARNING: DATA RACE
Read at 0x00c420098090 by goroutine 65:
  math/rand.(*Rand).Read()
      /usr/local/go/src/math/rand/rand.go:177 +0x6a
  math/rand.Read()
      /usr/local/go/src/math/rand/rand.go:248 +0x6c
  XXX/XXX_test.TestXXX.func2()
      /go/src/XXX/XXX_test.go:360 +0x188

Previous write at 0x00c420098090 by goroutine 66:
  math/rand.(*Rand).Read()
      /usr/local/go/src/math/rand/rand.go:188 +0x127
  math/rand.Read()
      /usr/local/go/src/math/rand/rand.go:248 +0x6c
  XXX/XXX_test.TestXXX.func2()
      /go/src/XXX/XXX_test.go:360 +0x188

Goroutine 65 (running) created at:
  XXX/XXX_test.TestXXX()
      /go/src/XXX/XXX_test.go:389 +0x8c2
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:610 +0xc9

Goroutine 66 (running) created at:
  XXX/XXX_test.TestXXX()
      /go/src/XXX/XXX_test.go:389 +0x8c2
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:610 +0xc9
==================

I can confirm it does not happen on 1.6, at least with my runs.

@yonderblue yonderblue changed the title False positive on race detector for math.Rand? False positive on race detector for math.Rand Jul 9, 2016
@dsnet dsnet added this to the Go1.7 milestone Jul 9, 2016
@dsnet
Copy link
Member

@dsnet dsnet commented Jul 9, 2016

/cc @adg @ianlancetaylor @randall77

The recent commit 8d966ba for #16124 has no locks over the readPos and readVal fields. The documentation for math/rand seems to indicate that concurrent calls of Read should be permitted.

We could use a sync.Mutex or atomic instructions to update those fields.

@dsnet dsnet changed the title False positive on race detector for math.Rand math/rand: use of Read is racy Jul 9, 2016
@gopherbot
Copy link

@gopherbot gopherbot commented Jul 9, 2016

CL https://golang.org/cl/24839 mentions this issue.

@gopherbot
Copy link

@gopherbot gopherbot commented Jul 10, 2016

CL https://golang.org/cl/24852 mentions this issue.

@gopherbot
Copy link

@gopherbot gopherbot commented Jul 11, 2016

CL https://golang.org/cl/24857 mentions this issue.

@gopherbot gopherbot closed this in fb3cf5c Jul 11, 2016
gopherbot pushed a commit that referenced this issue Jul 11, 2016
A follow-on to https://golang.org/cl/24852 that mentions the
documentation clarifications.

Updates #16308.

Change-Id: Ic2a6e1d4938d74352f93a6649021fb610efbfcd0
Reviewed-on: https://go-review.googlesource.com/24857
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
@golang golang locked and limited conversation to collaborators Jul 11, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.