testing: common.finished is accessed without holding common.mu lock

[maruel]

Note: CL to fix this is coming soon.

This triggers the race detector (for a good reason) when a test case fails by calling directly or indirectly t.FailNow() from two different goroutine.

Repro case

func TestRace(t *testing.T) {
  t2 := testing.T{}
  var wg sync.WaitGroup
  for i := 0; i < 2; i++ {
    wg.Add(1)
    go func() {
      defer wg.Done()
      t2.FailNow()
    }()
  }
  wg.Wait()
}

Actual

go test ./testing -race

==================
WARNING: DATA RACE
Write by goroutine 30:
  testing.(*common).FailNow()
      /home/maruel/src/golang/src/testing/testing.go:333 +0x48
  testing_test.TestRace.func1()
      /home/maruel/src/golang/src/testing/testing_test.go:28 +0x5d

Previous write by goroutine 31:
  testing.(*common).FailNow()
      /home/maruel/src/golang/src/testing/testing.go:333 +0x48
  testing_test.TestRace.func1()
      /home/maruel/src/golang/src/testing/testing_test.go:28 +0x5d

Goroutine 30 (running) created at:
  testing_test.TestRace()
      /home/maruel/src/golang/src/testing/testing_test.go:29 +0x10f
  testing.tRunner()
      /home/maruel/src/golang/src/testing/testing.go:452 +0xfc

Goroutine 31 (finished) created at:
  testing_test.TestRace()
      /home/maruel/src/golang/src/testing/testing_test.go:29 +0x10f
  testing.tRunner()
      /home/maruel/src/golang/src/testing/testing.go:452 +0xfc
==================
PASS
Found 1 data race(s)
FAIL    testing 2.480s

[bradfitz]

The docs say: "FailNow must be called from the goroutine running the test or benchmark function, not from other goroutines created during the test."

Also, you can't create your own *testing.T.

So, I don't think there's anything to fix here.

[gopherbot]

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