• Brad Fitzpatrick's avatar
    net/http: tighten protocol between Transport.roundTrip and persistConn.readLoop · 7fa98467
    Brad Fitzpatrick authored
    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: 's avatarAndrew Gerrand <adg@golang.org>
    7fa98467
Name
Last commit
Last update
..
archive Loading commit data...
bufio Loading commit data...
builtin Loading commit data...
bytes Loading commit data...
cmd Loading commit data...
compress Loading commit data...
container Loading commit data...
crypto Loading commit data...
database/sql Loading commit data...
debug Loading commit data...
encoding Loading commit data...
errors Loading commit data...
expvar Loading commit data...
flag Loading commit data...
fmt Loading commit data...
go Loading commit data...
hash Loading commit data...
html Loading commit data...
image Loading commit data...
index/suffixarray Loading commit data...
internal Loading commit data...
io Loading commit data...
log Loading commit data...
math Loading commit data...
mime Loading commit data...
net Loading commit data...
os Loading commit data...
path Loading commit data...
reflect Loading commit data...
regexp Loading commit data...
runtime Loading commit data...
sort Loading commit data...
strconv Loading commit data...
strings Loading commit data...
sync Loading commit data...
syscall Loading commit data...
testing Loading commit data...
text Loading commit data...
time Loading commit data...
unicode Loading commit data...
unsafe Loading commit data...
vendor/golang.org/x/net/http2/hpack Loading commit data...
Make.dist Loading commit data...
all.bash Loading commit data...
all.bat Loading commit data...
all.rc Loading commit data...
androidtest.bash Loading commit data...
bootstrap.bash Loading commit data...
buildall.bash Loading commit data...
clean.bash Loading commit data...
clean.bat Loading commit data...
clean.rc Loading commit data...
iostest.bash Loading commit data...
make.bash Loading commit data...
make.bat Loading commit data...
make.rc Loading commit data...
nacltest.bash Loading commit data...
race.bash Loading commit data...
race.bat Loading commit data...
run.bash Loading commit data...
run.bat Loading commit data...
run.rc Loading commit data...