Skip to content

Commit

Permalink
net/http: fix data race in test
Browse files Browse the repository at this point in the history
I can't reproduce the race, but this should fix it.

Fixes #8483

LGTM=dvyukov
R=dvyukov
CC=golang-codereviews
https://golang.org/cl/126610043
  • Loading branch information
bradfitz committed Aug 26, 2014
1 parent 87b4525 commit 32c0dce
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 17 deletions.
6 changes: 6 additions & 0 deletions src/pkg/net/http/export_test.go
Expand Up @@ -70,3 +70,9 @@ func ResetCachedEnvironment() {
}

var DefaultUserAgent = defaultUserAgent

// SetPendingDialHooks sets the hooks that run before and after handling
// pending dials.
func SetPendingDialHooks(before, after func()) {
prePendingDial, postPendingDial = before, after
}
19 changes: 15 additions & 4 deletions src/pkg/net/http/transport.go
Expand Up @@ -444,6 +444,9 @@ func (t *Transport) dial(network, addr string) (c net.Conn, err error) {
return net.Dial(network, addr)
}

// Testing hooks:
var prePendingDial, postPendingDial func()

// getConn dials and creates a new persistConn to the target as
// specified in the connectMethod. This includes doing a proxy CONNECT
// and/or setting up TLS. If this doesn't return an error, the persistConn
Expand All @@ -460,9 +463,17 @@ func (t *Transport) getConn(req *Request, cm connectMethod) (*persistConn, error
dialc := make(chan dialRes)

handlePendingDial := func() {
if v := <-dialc; v.err == nil {
t.putIdleConn(v.pc)
if prePendingDial != nil {
prePendingDial()
}
go func() {
if v := <-dialc; v.err == nil {
t.putIdleConn(v.pc)
}
if postPendingDial != nil {
postPendingDial()
}
}()
}

cancelc := make(chan struct{})
Expand All @@ -484,10 +495,10 @@ func (t *Transport) getConn(req *Request, cm connectMethod) (*persistConn, error
// else's dial that they didn't use.
// But our dial is still going, so give it away
// when it finishes:
go handlePendingDial()
handlePendingDial()
return pc, nil
case <-cancelc:
go handlePendingDial()
handlePendingDial()
return nil, errors.New("net/http: request canceled while waiting for connection")
}
}
Expand Down
24 changes: 11 additions & 13 deletions src/pkg/net/http/transport_test.go
Expand Up @@ -1063,20 +1063,18 @@ func TestTransportConcurrency(t *testing.T) {
var wg sync.WaitGroup
wg.Add(numReqs)

tr := &Transport{
Dial: func(netw, addr string) (c net.Conn, err error) {
// Due to the Transport's "socket late
// binding" (see idleConnCh in transport.go),
// the numReqs HTTP requests below can finish
// with a dial still outstanding. So count
// our dials as work too so the leak checker
// doesn't complain at us.
wg.Add(1)
defer wg.Done()
return net.Dial(netw, addr)
},
}
// Due to the Transport's "socket late binding" (see
// idleConnCh in transport.go), the numReqs HTTP requests
// below can finish with a dial still outstanding. To keep
// the leak checker happy, keep track of pending dials and
// wait for them to finish (and be closed or returned to the
// idle pool) before we close idle connections.
SetPendingDialHooks(func() { wg.Add(1) }, wg.Done)
defer SetPendingDialHooks(nil, nil)

tr := &Transport{}
defer tr.CloseIdleConnections()

c := &Client{Transport: tr}
reqs := make(chan string)
defer close(reqs)
Expand Down

0 comments on commit 32c0dce

Please sign in to comment.