code review 126610043: net/http: fix data race in test

net/http: fix data race in test

I can't reproduce the race, but this should fix it.

Fixes Issue 8483

[bradfitz, 2014-08-25 18:02:28]

Hello dvyukov@google.com (cc: golang-codereviews@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg/

[dvyukov, 2014-08-25 19:18:17]

https://codereview.appspot.com/126610043/diff/40001/src/pkg/net/http/transpor...
File src/pkg/net/http/transport_test.go (right):

https://codereview.appspot.com/126610043/diff/40001/src/pkg/net/http/transpor...
src/pkg/net/http/transport_test.go:1083: if done.v && err == nil {
I afraid a connection still can leak here.
Consider that a test goroutine picks up an idle conn, but the dial is still
kicked off and it finishes before done.v is set. But handlePendingDial goroutine
calls putIdleConn after tr.CloseIdleConnections.
This scenario leaves an idle conn.

[bradfitz, 2014-08-25 19:36:44]

PTAL



On Mon, Aug 25, 2014 at 12:18 PM, <dvyukov@google.com> wrote:

>
> https://codereview.appspot.com/126610043/diff/40001/src/
> pkg/net/http/transport_test.go
> File src/pkg/net/http/transport_test.go (right):
>
> https://codereview.appspot.com/126610043/diff/40001/src/
> pkg/net/http/transport_test.go#newcode1083
> src/pkg/net/http/transport_test.go:1083: if done.v && err == nil {
> I afraid a connection still can leak here.
> Consider that a test goroutine picks up an idle conn, but the dial is
> still kicked off and it finishes before done.v is set. But
> handlePendingDial goroutine calls putIdleConn after
> tr.CloseIdleConnections.
> This scenario leaves an idle conn.
>
> https://codereview.appspot.com/126610043/
>

[bradfitz, 2014-08-25 19:36:48]

Hello dvyukov@google.com (cc: golang-codereviews@googlegroups.com),

Please take another look.

[dvyukov, 2014-08-26 04:45:53]

LGTM

[bradfitz, 2014-08-26 04:50:48]

*** Submitted as https://code.google.com/p/go/source/detail?r=50ac9eded6ad ***

net/http: fix data race in test

I can't reproduce the race, but this should fix it.

Fixes Issue 8483

LGTM=dvyukov
R=dvyukov
CC=golang-codereviews
https://codereview.appspot.com/126610043

[gobot, 2014-08-28 01:16:12]

This CL appears to have broken the linux-386-387-temp builder.
See http://build.golang.org/log/fdde74740e4802c5853d83057cc368809a8e3997