From 07525e16ba9fcb8924ed872b015dc217d1b01b6b Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Thu, 6 Jan 2022 12:20:14 -0500 Subject: [PATCH] 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 Run-TryBot: Russ Cox Reviewed-by: Robert Griesemer --- src/math/big/rat.go | 2 +- src/math/big/rat_test.go | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/math/big/rat.go b/src/math/big/rat.go index d35cd4cbd1..731a979ff7 100644 --- 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 02569c1b16..d98c89b357 100644 --- 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 + } +} -- 2.50.0