[tailscale-cherrypick] net/http: fix two bugs from CL 828504 - #183
Merged
Merged
Conversation
CL 828504 ("net/http: don't pin bufio buffers on idle HTTP/1
keep-alive connections") broke keep-alive connections over net.Conn
implementations whose deadline errors don't wrap
os.ErrDeadlineExceeded, such as gVisor's gonet (used by Tailscale's
netstack). Its idle buffer release probe sets a short read deadline
and uses errors.Is(err, os.ErrDeadlineExceeded) to distinguish
"connection merely idle" from a real error, so a bespoke timeout error
caused the server to close every keep-alive connection about 50ms
after each response. Likewise in connReader.handleReadErrorLocked.
So use old-school net.Error checks instead for compatibility.
CL 828504 also introduced a second bug, present even with modern
net.Conn implementations: the probe's expected timeout error reached
connReader.handleReadErrorLocked, which canceled the connection-level
context that every subsequent request's context on that connection
derives from. Any request arriving after an idle period longer than
50ms thus began with an already-canceled Context.
Updates golang#80735
Updates #174
Change-Id: Ia69d79e8e1fc30aec9e716ed5691db7474b05f0e
Reviewed-on: https://go-review.googlesource.com/c/go/+/830364
Reviewed-by: Nicholas Husin <nsh@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Nicholas Husin <husin@google.com>
Reviewed-by: Davanum Srinivas (dims) <davanum@gmail.com>
(cherry picked from commit b35e1f9)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CL 828504 ("net/http: don't pin bufio buffers on idle HTTP/1
keep-alive connections") broke keep-alive connections over net.Conn
implementations whose deadline errors don't wrap
os.ErrDeadlineExceeded, such as gVisor's gonet (used by Tailscale's
netstack). Its idle buffer release probe sets a short read deadline
and uses errors.Is(err, os.ErrDeadlineExceeded) to distinguish
"connection merely idle" from a real error, so a bespoke timeout error
caused the server to close every keep-alive connection about 50ms
after each response. Likewise in connReader.handleReadErrorLocked.
So use old-school net.Error checks instead for compatibility.
CL 828504 also introduced a second bug, present even with modern
net.Conn implementations: the probe's expected timeout error reached
connReader.handleReadErrorLocked, which canceled the connection-level
context that every subsequent request's context on that connection
derives from. Any request arriving after an idle period longer than
50ms thus began with an already-canceled Context.
Updates golang#80735
Updates #174
Reviewed-on: https://go-review.googlesource.com/c/go/+/830364
Reviewed-by: Nicholas Husin nsh@golang.org
Reviewed-by: Dmitri Shuralyov dmitshur@google.com
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com golang-scoped@luci-project-accounts.iam.gserviceaccount.com
Reviewed-by: Nicholas Husin husin@google.com
Reviewed-by: Davanum Srinivas (dims) davanum@gmail.com
(cherry picked from commit b35e1f9)