runtime/race: bogus race reports

[mikioh]

See https://build.golang.org/log/ec042c758132274722706baf832f2570629dd322

BenchmarkClientServerParallelTLS64-4            ==================
WARNING: DATA RACE
Read at 0x00c4202bcd00 by goroutine 161:
  net/http.(*Transport).getConn()
      /tmp/workdir/go/src/net/http/transport.go:871 +0x648
  net/http.(*Transport).RoundTrip()
      /tmp/workdir/go/src/net/http/transport.go:372 +0x4a7
  net/http.send()
      /tmp/workdir/go/src/net/http/client.go:256 +0x2e6
  net/http.(*Client).send()
      /tmp/workdir/go/src/net/http/client.go:146 +0x1b7
  net/http.(*Client).doFollowingRedirects()
      /tmp/workdir/go/src/net/http/client.go:528 +0x3a5
  net/http.(*Client).Get()
      /tmp/workdir/go/src/net/http/client.go:418 +0xd0
  net/http_test.benchmarkClientServerParallel.func2()
      /tmp/workdir/go/src/net/http/serve_test.go:4221 +0x243
  testing.(*B).RunParallel.func1()
      /tmp/workdir/go/src/testing/benchmark.go:598 +0x150

Previous write at 0x00c4202bcd00 by goroutine 23:
  net/http.NewRequest()
      /tmp/workdir/go/src/net/http/request.go:694 +0x27a
  net/http.(*Client).Get()
      /tmp/workdir/go/src/net/http/client.go:414 +0x63
  net/http_test.benchmarkClientServerParallel.func2()
      /tmp/workdir/go/src/net/http/serve_test.go:4221 +0x243
  testing.(*B).RunParallel.func1()
      /tmp/workdir/go/src/testing/benchmark.go:598 +0x150

Goroutine 161 (running) created at:
  testing.(*B).RunParallel()
      /tmp/workdir/go/src/testing/benchmark.go:599 +0x270
  net/http_test.benchmarkClientServerParallel()
      /tmp/workdir/go/src/net/http/serve_test.go:4237 +0x165
  net/http_test.BenchmarkClientServerParallelTLS64()
      /tmp/workdir/go/src/net/http/serve_test.go:4196 +0x3c
  testing.(*B).runN()
      /tmp/workdir/go/src/testing/benchmark.go:139 +0xec
  testing.(*B).launch()
      /tmp/workdir/go/src/testing/benchmark.go:277 +0x1a2

Goroutine 23 (running) created at:
  testing.(*B).RunParallel()
      /tmp/workdir/go/src/testing/benchmark.go:599 +0x270
  net/http_test.benchmarkClientServerParallel()
      /tmp/workdir/go/src/net/http/serve_test.go:4237 +0x165
  net/http_test.BenchmarkClientServerParallelTLS64()
      /tmp/workdir/go/src/net/http/serve_test.go:4196 +0x3c
  testing.(*B).runN()
      /tmp/workdir/go/src/testing/benchmark.go:139 +0xec
  testing.(*B).launch()
      /tmp/workdir/go/src/testing/benchmark.go:277 +0x1a2
==================
==================
WARNING: DATA RACE
Read at 0x00c4202bcd10 by goroutine 161:
  net/http.(*Transport).getConn()
      /tmp/workdir/go/src/net/http/transport.go:874 +0x673
  net/http.(*Transport).RoundTrip()
      /tmp/workdir/go/src/net/http/transport.go:372 +0x4a7
  net/http.send()
      /tmp/workdir/go/src/net/http/client.go:256 +0x2e6
  net/http.(*Client).send()
      /tmp/workdir/go/src/net/http/client.go:146 +0x1b7
  net/http.(*Client).doFollowingRedirects()
      /tmp/workdir/go/src/net/http/client.go:528 +0x3a5
  net/http.(*Client).Get()
      /tmp/workdir/go/src/net/http/client.go:418 +0xd0
  net/http_test.benchmarkClientServerParallel.func2()
      /tmp/workdir/go/src/net/http/serve_test.go:4221 +0x243
  testing.(*B).RunParallel.func1()
      /tmp/workdir/go/src/testing/benchmark.go:598 +0x150

Previous write at 0x00c4202bcd10 by goroutine 23:
  net/http.NewRequest()
      /tmp/workdir/go/src/net/http/request.go:694 +0x27a
  net/http.(*Client).Get()
      /tmp/workdir/go/src/net/http/client.go:414 +0x63
  net/http_test.benchmarkClientServerParallel.func2()
      /tmp/workdir/go/src/net/http/serve_test.go:4221 +0x243
  testing.(*B).RunParallel.func1()
      /tmp/workdir/go/src/testing/benchmark.go:598 +0x150

Goroutine 161 (running) created at:
  testing.(*B).RunParallel()
      /tmp/workdir/go/src/testing/benchmark.go:599 +0x270
  net/http_test.benchmarkClientServerParallel()
      /tmp/workdir/go/src/net/http/serve_test.go:4237 +0x165
  net/http_test.BenchmarkClientServerParallelTLS64()
      /tmp/workdir/go/src/net/http/serve_test.go:4196 +0x3c
  testing.(*B).runN()
      /tmp/workdir/go/src/testing/benchmark.go:139 +0xec
  testing.(*B).launch()
      /tmp/workdir/go/src/testing/benchmark.go:277 +0x1a2

Goroutine 23 (running) created at:
  testing.(*B).RunParallel()
      /tmp/workdir/go/src/testing/benchmark.go:599 +0x270
  net/http_test.benchmarkClientServerParallel()
      /tmp/workdir/go/src/net/http/serve_test.go:4237 +0x165
  net/http_test.BenchmarkClientServerParallelTLS64()
      /tmp/workdir/go/src/net/http/serve_test.go:4196 +0x3c
  testing.(*B).runN()
      /tmp/workdir/go/src/testing/benchmark.go:139 +0xec
  testing.(*B).launch()
      /tmp/workdir/go/src/testing/benchmark.go:277 +0x1a2
==================

[mikioh]

On freebsd-amd64-race builder: https://build.golang.org/log/74ead1f381092682cd2985b9c91ccf448568f60c

[bradfitz]

Those first two stacks make no sense to me:

BenchmarkClientServerParallelTLS64-4            ==================
WARNING: DATA RACE
Read at 0x00c4202bcd00 by goroutine 161:
  net/http.(*Transport).getConn()
      /tmp/workdir/go/src/net/http/transport.go:871 +0x648
  net/http.(*Transport).RoundTrip()
      /tmp/workdir/go/src/net/http/transport.go:372 +0x4a7
  net/http.send()
      /tmp/workdir/go/src/net/http/client.go:256 +0x2e6
  net/http.(*Client).send()
      /tmp/workdir/go/src/net/http/client.go:146 +0x1b7
  net/http.(*Client).doFollowingRedirects()
      /tmp/workdir/go/src/net/http/client.go:528 +0x3a5
  net/http.(*Client).Get()
      /tmp/workdir/go/src/net/http/client.go:418 +0xd0
  net/http_test.benchmarkClientServerParallel.func2()
      /tmp/workdir/go/src/net/http/serve_test.go:4221 +0x243
  testing.(*B).RunParallel.func1()
      /tmp/workdir/go/src/testing/benchmark.go:598 +0x150

Previous write at 0x00c4202bcd00 by goroutine 23:
  net/http.NewRequest()
      /tmp/workdir/go/src/net/http/request.go:694 +0x27a
  net/http.(*Client).Get()
      /tmp/workdir/go/src/net/http/client.go:414 +0x63
  net/http_test.benchmarkClientServerParallel.func2()
      /tmp/workdir/go/src/net/http/serve_test.go:4221 +0x243
  testing.(*B).RunParallel.func1()
      /tmp/workdir/go/src/testing/benchmark.go:598 +0x150

I'm not sure how to debug this. @dvyukov? Is this correct?

[bradfitz]

I got another bogus one,
https://storage.googleapis.com/go-build-log/f48ca822/linux-amd64-race_04ff3748.log

Here's why it's bogus:

The race says:

Previous write at 0x00c42054ce20 by goroutine 235:
  net/http.readRequest()
      /tmp/workdir/go/src/net/http/request.go:827 +0x402
  net/http.(*conn).readRequest()
      /tmp/workdir/go/src/net/http/server.go:765 +0x233
  net/http.(*conn).serve()
      /tmp/workdir/go/src/net/http/server.go:1508 +0x3ff

But here's the code, with line 827 at the bottom highlighted:

func readRequest(b *bufio.Reader, deleteHostHeader bool) (req *Request, err error) {
        tp := newTextprotoReader(b)
        req = new(Request)

        // First line: GET /index.html HTTP/1.0                                                                                                     
        var s string
        if s, err = tp.ReadLine(); err != nil {
                return nil, err
        }
        defer func() {
                putTextprotoReader(tp)
                if err == io.EOF {
                        err = io.ErrUnexpectedEOF
                }
        }()

        var ok bool
        req.Method, req.RequestURI, req.Proto, ok = parseRequestLine(s)
        if !ok {
                return nil, &badStringError{"malformed HTTP request", s}
        }
        rawurl := req.RequestURI
        if req.ProtoMajor, req.ProtoMinor, ok = ParseHTTPVersion(req.Proto); !ok {
                return nil, &badStringError{"malformed HTTP version", req.Proto}
        }

        // CONNECT requests are used two different ways, and neither uses a full URL:                                                               
        // The standard use is to tunnel HTTPS through an HTTP proxy.                                                                               
        // It looks like "CONNECT www.google.com:443 HTTP/1.1", and the parameter is                                                                
        // just the authority section of a URL. This information should go in req.URL.Host.                                                         
        //                                                                                                                                          
        // The net/rpc package also uses CONNECT, but there the parameter is a path                                                                 
        // that starts with a slash. It can be parsed with the regular URL parser,                                                                  
        // and the path will end up in req.URL.Path, where it needs to be in order for                                                              
        // RPC to work.                                                                                                                             
        justAuthority := req.Method == "CONNECT" && !strings.HasPrefix(rawurl, "/")
        if justAuthority {
                rawurl = "http://" + rawurl
        }

        if req.URL, err = url.ParseRequestURI(rawurl); err != nil {   // <----- LINE 827
                return nil, err
        }

That is all in a single goroutine, and it creates the memory (on the second line of the function), before assigning to it.

It can't race with anything yet, because that memory hasn't even escaped.

@dvyukov, up to you to debug.

[rsc]

@aclements, did this start at the same time as #15747 (with Keith's leak fix)?

[aclements]

@rsc, hard to say (and hard to grep for). I only see two on the dashboard since the beginning of the year:

2016-05-18T20:44:00-075880a/linux-amd64-race
2016-05-19T03:51:11-255e206/freebsd-amd64-race

[dvyukov]

Here:
https://storage.googleapis.com/go-build-log/f48ca822/linux-amd64-race_04ff3748.log
the benchmark itself failed with:

--- BENCH: BenchmarkClientServerParallelTLS64-4
    serve_test.go:4237: Get: Get /: http: Request.RequestURI can't be set in client requests.

right after race reports on URI object. That's concerning. It may be a bug affecting non-race build. Do we still have any unfixed memory corruptions?