math/big: fix spurious race in Rat.Denom, Float.SetRat

Rat maintains the invariant that x.b.neg is always false,
but Rat.Denom was writing x.b.neg = false itself too.
That makes Rat.Denom a writing operation, when it should
be a read-only operation. That in turn makes it unsafe to
use from multiple goroutines, which is highly unexpected.
Make it read-only and therefore race-free again.

Fixes #50473.

Change-Id: I97b87913954511e5200c0665d16b9ed63422e505
Reviewed-on: https://go-review.googlesource.com/c/go/+/375935
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>

diff --git a/src/math/big/rat.go b/src/math/big/rat.go
index d35cd4cbd10c6ff74e894a4fd600f2ad6866f1ce..731a979ff7a18896a0acfbfc2467e190063b2814 100644 (file)
--- a/src/math/big/rat.go
+++ b/src/math/big/rat.go
@@ -418,7 +418,7 @@ func (x *Rat) Num() *Int {
 // If the result is a reference to x's denominator it
 // may change if a new value is assigned to x, and vice versa.
 func (x *Rat) Denom() *Int {
-       x.b.neg = false // the result is always >= 0
+       // Note that x.b.neg is guaranteed false.
        if len(x.b.abs) == 0 {
                // Note: If this proves problematic, we could
                //       panic instead and require the Rat to

diff --git a/src/math/big/rat_test.go b/src/math/big/rat_test.go
index 02569c1b16a33fc90a5e7a81febcc0f75394aab0..d98c89b3578a7aee1e88e62411c6b192e61cce57 100644 (file)
--- a/src/math/big/rat_test.go
+++ b/src/math/big/rat_test.go
@@ -726,3 +726,21 @@ func TestIssue34919(t *testing.T) {
                }
        }
 }
+
+func TestDenomRace(t *testing.T) {
+       x := NewRat(1, 2)
+       const N = 3
+       c := make(chan bool, N)
+       for i := 0; i < N; i++ {
+               go func() {
+                       // Denom (also used by Float.SetRat) used to mutate x unnecessarily,
+                       // provoking race reports when run in the race detector.
+                       x.Denom()
+                       new(Float).SetRat(x)
+                       c <- true
+               }()
+       }
+       for i := 0; i < N; i++ {
+               <-c
+       }
+}