• Brad Fitzpatrick's avatar
    net/http: fix Transport race returning bodyless responses and reusing conns · 1fe39339
    Brad Fitzpatrick authored
    The Transport had a delicate protocol between its readLoop goroutine
    and the goroutine calling RoundTrip. The basic concern is that the
    caller's RoundTrip goroutine wants to wait for either a
    connection-level error (the conn dying) or the response. But sometimes
    both happen: there's a valid response (without a body), but the conn
    is also going away. Both goroutines' logic dealing with this had grown
    large and complicated with hard-to-follow comments over the years.
    
    Simplify and document. Pull some bits into functions and do all
    bodyless stuff in one place (it's special enough), rather than having
    a bunch of conditionals scattered everywhere. One test is no longer
    even applicable since the race it tested is no longer possible (the
    code doesn't exist).
    
    The bug that this fixes is that when the Transport reads a bodyless
    response from a server, it was returning that response before
    returning the persistent connection to the idle pool. As a result,
    ~1/1000 of serial requests would end up creating a new connection
    rather than re-using the just-used connection due to goroutine
    scheduling chance. Instead, this now adds bodyless responses'
    connections back to the idle pool first, then sends the response to
    the RoundTrip goroutine, but making sure that the RoundTrip goroutine
    is outside of its select on the connection dying.
    
    There's a new buffered channel involved now, which is a minor
    complication, but it's much more self-contained and well-documented
    than the previous complexity. (The alternative of making the
    responseAndError channel itself unbuffered is too invasive and risky
    at this point; it would require a number of changes to avoid
    deadlocked goroutines in error cases)
    
    In any case, flakes look to be gone now. We'll see if trybots agree.
    
    Fixes #13633
    
    Change-Id: I95a22942b2aa334ae7c87331fddd751d4cdfdffc
    Reviewed-on: https://go-review.googlesource.com/17890Reviewed-by: 's avatarRuss Cox <rsc@golang.org>
    Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
    1fe39339
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...