From 613c39d390d0672bb2e67c5d2266e07491e88582 Mon Sep 17 00:00:00 2001 From: B Tasker <88340935+btasker@users.noreply.github.com> Date: Tue, 18 Apr 2023 13:23:52 +0100 Subject: [PATCH 1/2] net/http2: if the only stream in a connection times out, prevent re-use This commit mitigates some raciness which can occur when an established connection fails. Without it, the connection can continue to be drawn from the connection pool, leading to subsequent requests failing. Fixes golang/go#59690 --- http2/transport.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/http2/transport.go b/http2/transport.go index f965579f7d..c577b5def9 100644 --- a/http2/transport.go +++ b/http2/transport.go @@ -1444,8 +1444,22 @@ func (cs *clientStream) writeRequest(req *http.Request) (err error) { respHeaderRecv = nil respHeaderTimer = nil // keep waiting for END_STREAM case <-cs.abort: + // If this was the only active stream, mark the connection + // as not for re-use in order to address raciness if the caller + // tries to call closeIdleConnections() before the stream has been + // removed + if len(cc.streams) == 1 { + cc.doNotReuse = true + } return cs.abortErr case <-ctx.Done(): + // If this was the only active stream, mark the connection + // as not for re-use in order to address raciness if the caller + // tries to call closeIdleConnections() before the stream has been + // removed + if len(cc.streams) == 1 { + cc.doNotReuse = true + } return ctx.Err() case <-cs.reqCancel: return errRequestCanceled From ab7b5a39c5def296519560559b05ed29d2b4c6d1 Mon Sep 17 00:00:00 2001 From: B Tasker <88340935+btasker@users.noreply.github.com> Date: Wed, 19 Apr 2023 09:45:30 +0100 Subject: [PATCH 2/2] net/http2: Hold the cc.mu Mutex whilst changing doNotReuse --- http2/transport.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/http2/transport.go b/http2/transport.go index c577b5def9..0f0edc5fe3 100644 --- a/http2/transport.go +++ b/http2/transport.go @@ -1449,7 +1449,9 @@ func (cs *clientStream) writeRequest(req *http.Request) (err error) { // tries to call closeIdleConnections() before the stream has been // removed if len(cc.streams) == 1 { + cc.mu.Lock() cc.doNotReuse = true + cc.mu.Unlock() } return cs.abortErr case <-ctx.Done(): @@ -1458,7 +1460,9 @@ func (cs *clientStream) writeRequest(req *http.Request) (err error) { // tries to call closeIdleConnections() before the stream has been // removed if len(cc.streams) == 1 { + cc.mu.Lock() cc.doNotReuse = true + cc.mu.Unlock() } return ctx.Err() case <-cs.reqCancel: