mirror of
https://github.com/golang/go.git
synced 2025-05-17 13:24:38 +00:00
In debugging the flaky test in #13825, I discovered that my previous change to tighten and simplify the communication protocol between Transport.roundTrip and persistConn.readLoop in https://golang.org/cl/17890 wasn't complete. This change simplifies it further: the buffered-vs-unbuffered complexity goes away, and we no longer need to re-try channel reads in the select case. It was trying to prioritize channels in the case that two were readable in the select. (it was only failing in the race builder because the race builds randomize select scheduling) The problem was that in the bodyless response case we had to return the idle connection before replying to roundTrip. But putIdleConn previously both added it to the free list (which we wanted), but also closed the connection, which made the caller goroutine (Transport.roundTrip) have two readable cases: pc.closech, and the response. We guarded against similar conditions in the caller's select for two readable channels, but such a fix wasn't possible here, and would be overly complicated. Instead, switch to unbuffered channels. The unbuffered channels were only to prevent goroutine leaks, so address that differently: add a "callerGone" channel closed by the caller on exit, and select on that during any unbuffered sends. As part of the fix, split putIdleConn into two halves: a part that just returns to the freelist, and a part that also closes. Update the four callers to the variants each wanted. Incidentally, the connections were closing on return to the pool due to MaxIdleConnsPerHost (somewhat related: #13801), but this bug could've manifested for plenty of other reasons. Fixes #13825 Change-Id: I6fa7136e2c52909d57a22ea4b74d0155fdf0e6fa Reviewed-on: https://go-review.googlesource.com/18282 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Andrew Gerrand <adg@golang.org>
146 lines
3.5 KiB
Go
146 lines
3.5 KiB
Go
// Copyright 2011 The Go Authors. All rights reserved.
|
|
// Use of this source code is governed by a BSD-style
|
|
// license that can be found in the LICENSE file.
|
|
|
|
// Bridge package to expose http internals to tests in the http_test
|
|
// package.
|
|
|
|
package http
|
|
|
|
import (
|
|
"net"
|
|
"sync"
|
|
"time"
|
|
)
|
|
|
|
var (
|
|
DefaultUserAgent = defaultUserAgent
|
|
NewLoggingConn = newLoggingConn
|
|
ExportAppendTime = appendTime
|
|
ExportRefererForURL = refererForURL
|
|
ExportServerNewConn = (*Server).newConn
|
|
ExportCloseWriteAndWait = (*conn).closeWriteAndWait
|
|
ExportErrRequestCanceled = errRequestCanceled
|
|
ExportServeFile = serveFile
|
|
ExportHttp2ConfigureTransport = http2ConfigureTransport
|
|
ExportHttp2ConfigureServer = http2ConfigureServer
|
|
)
|
|
|
|
func init() {
|
|
// We only want to pay for this cost during testing.
|
|
// When not under test, these values are always nil
|
|
// and never assigned to.
|
|
testHookMu = new(sync.Mutex)
|
|
}
|
|
|
|
var (
|
|
SetEnterRoundTripHook = hookSetter(&testHookEnterRoundTrip)
|
|
SetTestHookWaitResLoop = hookSetter(&testHookWaitResLoop)
|
|
SetRoundTripRetried = hookSetter(&testHookRoundTripRetried)
|
|
)
|
|
|
|
func SetReadLoopBeforeNextReadHook(f func()) {
|
|
testHookMu.Lock()
|
|
defer testHookMu.Unlock()
|
|
unnilTestHook(&f)
|
|
testHookReadLoopBeforeNextRead = f
|
|
}
|
|
|
|
// SetPendingDialHooks sets the hooks that run before and after handling
|
|
// pending dials.
|
|
func SetPendingDialHooks(before, after func()) {
|
|
unnilTestHook(&before)
|
|
unnilTestHook(&after)
|
|
testHookPrePendingDial, testHookPostPendingDial = before, after
|
|
}
|
|
|
|
func SetTestHookServerServe(fn func(*Server, net.Listener)) { testHookServerServe = fn }
|
|
|
|
func NewTestTimeoutHandler(handler Handler, ch <-chan time.Time) Handler {
|
|
return &timeoutHandler{
|
|
handler: handler,
|
|
timeout: func() <-chan time.Time { return ch },
|
|
// (no body and nil cancelTimer)
|
|
}
|
|
}
|
|
|
|
func ResetCachedEnvironment() {
|
|
httpProxyEnv.reset()
|
|
httpsProxyEnv.reset()
|
|
noProxyEnv.reset()
|
|
}
|
|
|
|
func (t *Transport) NumPendingRequestsForTesting() int {
|
|
t.reqMu.Lock()
|
|
defer t.reqMu.Unlock()
|
|
return len(t.reqCanceler)
|
|
}
|
|
|
|
func (t *Transport) IdleConnKeysForTesting() (keys []string) {
|
|
keys = make([]string, 0)
|
|
t.idleMu.Lock()
|
|
defer t.idleMu.Unlock()
|
|
if t.idleConn == nil {
|
|
return
|
|
}
|
|
for key := range t.idleConn {
|
|
keys = append(keys, key.String())
|
|
}
|
|
return
|
|
}
|
|
|
|
func (t *Transport) IdleConnCountForTesting(cacheKey string) int {
|
|
t.idleMu.Lock()
|
|
defer t.idleMu.Unlock()
|
|
if t.idleConn == nil {
|
|
return 0
|
|
}
|
|
for k, conns := range t.idleConn {
|
|
if k.String() == cacheKey {
|
|
return len(conns)
|
|
}
|
|
}
|
|
return 0
|
|
}
|
|
|
|
func (t *Transport) IdleConnChMapSizeForTesting() int {
|
|
t.idleMu.Lock()
|
|
defer t.idleMu.Unlock()
|
|
return len(t.idleConnCh)
|
|
}
|
|
|
|
func (t *Transport) IsIdleForTesting() bool {
|
|
t.idleMu.Lock()
|
|
defer t.idleMu.Unlock()
|
|
return t.wantIdle
|
|
}
|
|
|
|
func (t *Transport) RequestIdleConnChForTesting() {
|
|
t.getIdleConnCh(connectMethod{nil, "http", "example.com"})
|
|
}
|
|
|
|
func (t *Transport) PutIdleTestConn() bool {
|
|
c, _ := net.Pipe()
|
|
return t.tryPutIdleConn(&persistConn{
|
|
t: t,
|
|
conn: c, // dummy
|
|
closech: make(chan struct{}), // so it can be closed
|
|
cacheKey: connectMethodKey{"", "http", "example.com"},
|
|
}) == nil
|
|
}
|
|
|
|
// All test hooks must be non-nil so they can be called directly,
|
|
// but the tests use nil to mean hook disabled.
|
|
func unnilTestHook(f *func()) {
|
|
if *f == nil {
|
|
*f = nop
|
|
}
|
|
}
|
|
|
|
func hookSetter(dst *func()) func(func()) {
|
|
return func(fn func()) {
|
|
unnilTestHook(&fn)
|
|
*dst = fn
|
|
}
|
|
}
|