net/http: data race in test

[dvyukov]

Freebsd-race builder says:

http://build.golang.org/log/08f6845ff56ba26e380d39f678275ed03072ba19

==================
WARNING: DATA RACE
Read by goroutine 61:
  sync.raceRead()
      /usr/local/go/src/pkg/sync/race.go:37 +0x35
  sync.(*WaitGroup).Add()
      /usr/local/go/src/pkg/sync/waitgroup.go:60 +0xbe
  net/http_test.func·189()
      /usr/home/gopher/racer/work/freebsd-amd64-race-98c597959808/go/src/pkg/net/http/transport_test.go:1074
+0xa6
  net/http.(*Transport).dial()
      /usr/home/gopher/racer/work/freebsd-amd64-race-98c597959808/go/src/pkg/net/http/transport.go:442
+0xcb
  net/http.(*Transport).dialConn()
      /usr/home/gopher/racer/work/freebsd-amd64-race-98c597959808/go/src/pkg/net/http/transport.go:496
+0xab
  net/http.func·022()
      /usr/home/gopher/racer/work/freebsd-amd64-race-98c597959808/go/src/pkg/net/http/transport.go:472
+0x8c

Previous write by goroutine 50:
  sync.raceWrite()
      /usr/local/go/src/pkg/sync/race.go:41 +0x35
  sync.(*WaitGroup).Wait()
      /usr/local/go/src/pkg/sync/waitgroup.go:122 +0x176
  net/http_test.TestTransportConcurrency()
      /usr/home/gopher/racer/work/freebsd-amd64-race-98c597959808/go/src/pkg/net/http/transport_test.go:1110
+0x6a3
  testing.tRunner()
      /usr/local/go/src/pkg/testing/testing.go:427 +0x10b

Goroutine 61 (running) created at:
  net/http.(*Transport).getConn()
      /usr/home/gopher/racer/work/freebsd-amd64-race-98c597959808/go/src/pkg/net/http/transport.go:474
+0x334
  net/http.(*Transport).RoundTrip()
      /usr/home/gopher/racer/work/freebsd-amd64-race-98c597959808/go/src/pkg/net/http/transport.go:201
+0x57e
  net/http.send()
      /usr/home/gopher/racer/work/freebsd-amd64-race-98c597959808/go/src/pkg/net/http/client.go:195
+0x626
  net/http.(*Client).send()
      /usr/home/gopher/racer/work/freebsd-amd64-race-98c597959808/go/src/pkg/net/http/client.go:118
+0x1ff
  net/http.(*Client).doFollowingRedirects()
      /usr/home/gopher/racer/work/freebsd-amd64-race-98c597959808/go/src/pkg/net/http/client.go:343
+0xd27
  net/http.(*Client).Get()
      /usr/home/gopher/racer/work/freebsd-amd64-race-98c597959808/go/src/pkg/net/http/client.go:275
+0xcd
  net/http_test.func·190()
      /usr/home/gopher/racer/work/freebsd-amd64-race-98c597959808/go/src/pkg/net/http/transport_test.go:1087
+0x188

Goroutine 50 (finished) created at:
  testing.RunTests()
      /usr/local/go/src/pkg/testing/testing.go:509 +0xb3e
  testing.Main()
      /usr/local/go/src/pkg/testing/testing.go:440 +0xa2
  main.main()
      net/http/_test/_testmain.go:479 +0xdc
==================

The test expects that Dial function will be called synchronously inside of client.Get.
But it is not. So the test does not wait for some of the dialers.

It could have been the root cause of:
https://golang.org/issue/6970
and some other spurious failures in net/http tests.

[bradfitz]

Comment 1:

CL 126610043

Owner changed to @bradfitz.

Status changed to Started.

[gopherbot]

Comment 2:

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

[bradfitz]

Comment 3:

This issue was closed by revision 32c0dce.

Status changed to Fixed.

[dvyukov]

Comment 4:

Brad, I think that the race detector actually pointed to a more serious not-test-only
issue.
So if a user creates a http.Client, issues a bunch of requests and then wants to
shutdown it and all opened connections; what is she intended to do? The report suggests
that just waiting for all pending requests and calling CloseIdleConnections won't do, as
there can be new racing connections. Obviously she can't do what you've done in the
test, as it uses the unexported function.
If this happens periodically, it can lead to serious resource leaks (the transport is
also preserved alive).
Am I missing something?

Status changed to New.

[rsc]

Comment 5:

Brad, can you look at Dmitriy's question?

Status changed to Accepted.

[gopherbot]

Comment 6:

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

[bradfitz]

Comment 7:

This issue was closed by revision f13cec9.

Status changed to Fixed.