runtime/race: false positive in race detector

[bradfitz]

I'm seeing what appears to be a false positive in the race detector on Linux in the x/net/http2 tests:

=== RUN   TestServerWithCurl_LenientCipherSuites
==================
WARNING: DATA RACE
Write by goroutine 40:
  sync/atomic.StoreInt32()
      /home/bradfitz/go/src/runtime/race_amd64.s:215 +0xb
  golang.org/x/net/http2.testServerWithCurl.func2()
      /home/bradfitz/src/golang.org/x/net/http2/server_test.go:2228 +0x35
  golang.org/x/net/http2.ConfigureServer.func1()
      /home/bradfitz/src/golang.org/x/net/http2/server.go:191 +0x67
  net/http.(*conn).serve()
      /home/bradfitz/go/src/net/http/server.go:1323 +0xa95

Previous write by goroutine 31:
  golang.org/x/net/http2.testServerWithCurl()
      /home/bradfitz/src/golang.org/x/net/http2/server_test.go:2227 +0x343
  golang.org/x/net/http2.TestServerWithCurl_LenientCipherSuites()
      /home/bradfitz/src/golang.org/x/net/http2/server_test.go:2204 +0x33
  testing.tRunner()
      /home/bradfitz/go/src/testing/testing.go:456 +0xdc

Goroutine 40 (running) created at:
  net/http.(*Server).Serve()
      /home/bradfitz/go/src/net/http/server.go:1930 +0x533
  net/http/httptest.(*Server).goServe.func1()
      /home/bradfitz/go/src/net/http/httptest/server.go:215 +0xac

Goroutine 31 (running) created at:
  testing.RunTests()
      /home/bradfitz/go/src/testing/testing.go:561 +0xaaf
  testing.(*M).Run()
      /home/bradfitz/go/src/testing/testing.go:494 +0xe4
  main.main()
      golang.org/x/net/http2/_test/_testmain.go:272 +0x210
==================
--- PASS: TestServerWithCurl_LenientCipherSuites (0.78s)
        server_test.go:2230: Running test server for curl to hit at: https://127.0.0.1:37619
PASS

Except those two write lines, 2227 and 2228 are:

        var gotConn int32
        testHookOnConn = func() { atomic.StoreInt32(&gotConn, 1) }

Is the implicit zeroing of that variable counting as a write?

The only reference to that variable is at the end of the test:

        if atomic.LoadInt32(&gotConn) == 0 {
                t.Error("never saw an http2 connection")
        }

/cc @randall77

[dvyukov]

The test starts server and after that sets testHookOnConn. It is OK since curl is not started yet, but race detector does not see the synchronization behind starting curl and accepting a new connection. Can you just move setup of testHookOnConn above start of the server? It should get rid of the race. Can't test it as it requires some docker magic that does not work on my machine.

However, there seems to be a false negative there. If race detector sees a race here:

var gotConn int32
testHookOnConn = func() { atomic.StoreInt32(&gotConn, 1) }

then it probably should also see a race on testHookOnConn var.

[dvyukov]

Are you sure you don't see a race on testHookOnConn?
Race detector has similar tests and races on func vars are reported. I've modeled what [race detector thinks] happens in the test, and it detects races on both x and testf:

var testf func()
func TestRaceGlobalFuncVariableRW(t *testing.T) {
    ch := make(chan bool, 1)
    go func() {
        if testf != nil {
            testf()
        }
    }()
    var x int32
    testf = func() {
        atomic.AddInt32(&x, 1)
    }
    ch <- true
    <-ch
}

[rsc]

I agree with Dmitriy that this is an accurate race report. Move the setup above ConfigureServer.

I also agree that you should be seeing a race on testHookOnConn itself. Do you?