Skip to content

Commit

Permalink
fix net/http caching of broken persistent connections
Browse files Browse the repository at this point in the history
The net/http transport code is currently broken, it keeps broken
persistent connections in the cache if a write error happens during
h2 handshake.

This is documented in the upstream bug at:
golang/go#40213

The problem occurs because in the "go" compiler the http2 code is
imported into http as a bundle, with an additional "http2" prefix
applied.  This messes up the erringRoundTripper handling because
the name doesn't match.

The solution is to have the "go" compiler look for an interface
instead, so we add a new dummy function that doesn't actually do
anything and then the "go" compiler can check whether the specified
RoundTripper implements the dummy function.

This is slightly different from the proposed upstream fixes for the
above upstream bug, it more closely follows how the equivalent
problem was solved by IsHTTP2NoCachedConnError().

Change-Id: Ia6e91acb15ff4fe996c8ea9b8a1032cede6c4aab
Partial-Bug: 1887438
Signed-off-by: Chris Friesen <[email protected]>
  • Loading branch information
cbf123 committed Mar 15, 2021
1 parent 49e4df5 commit 5f0f710
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
diff --git a/src/net/http/h2_bundle.go b/src/net/http/h2_bundle.go
index e9a55f3..6ec0ea1 100644
--- a/src/net/http/h2_bundle.go
+++ b/src/net/http/h2_bundle.go
@@ -8893,6 +8893,7 @@ func http2strSliceContains(ss []string, s string) bool {

type http2erringRoundTripper struct{ err error }

+func (rt http2erringRoundTripper) IsHTTP2ErringRoundtripper() {}
func (rt http2erringRoundTripper) RoundTrip(*Request) (*Response, error) { return nil, rt.err }

// gzipReader wraps a response body so it can lazily
diff --git a/src/net/http/transport.go b/src/net/http/transport.go
index db8ec4b..e3f9553 100644
--- a/src/net/http/transport.go
+++ b/src/net/http/transport.go
@@ -539,8 +539,7 @@ func (t *Transport) roundTrip(req *Request) (*Response, error) {
}

// Failed. Clean up and determine whether to retry.
-
- _, isH2DialError := pconn.alt.(http2erringRoundTripper)
+ _, isH2DialError := pconn.alt.(interface{ IsHTTP2ErringRoundtripper() })
if http2isNoCachedConnError(err) || isH2DialError {
if t.removeIdleConn(pconn) {
t.decConnsPerHost(pconn.cacheKey)
2 changes: 2 additions & 0 deletions languages/golang/centos/golang.spec
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ Requires: go-srpm-macros

Patch1: 0001-Don-t-use-the-bundled-tzdata-at-runtime-except-for-t.patch
Patch2: 0002-syscall-expose-IfInfomsg.X__ifi_pad-on-s390x.patch
Patch3: fix_http2_erringroundtripper_handling.patch

# Having documentation separate was broken
Obsoletes: %{name}-docs < 1.1-4
Expand Down Expand Up @@ -308,6 +309,7 @@ Requires: %{name} = %{version}-%{release}

%patch1 -p1
%patch2 -p1
%patch3 -p1

cp %{SOURCE1} ./src/runtime/

Expand Down

0 comments on commit 5f0f710

Please sign in to comment.