Stowage/go
2
0
Fork 0
mirror of https://github.com/golang/go.git synced 2026-06-28 03:40:37 +00:00
Code Issues Projects Releases Packages Wiki Activity

net/http/internal/http2: close client conn on GOAWAY with no reqs in-flight

Browse source 
When a client connection receives a GOAWAY and there are no in-flight
requests, we should immediately close the connection. Previously,
we'd only close it if there was at least one in-flight request:
the request would be canceled, and the connection would be closed
while cleaning up the request.

Fixes flaky net/http test TestServerKeepAlivesEnabled/h2, which fails
when the GOAWAY arrives after the connection's request has completed,
causing the client to skip closing the connection.

Includes a small refactor: Move ClientConn.setGoAway into
its only caller.

Fixes #68440

Change-Id: Ib234a7f15f19803dbd89f323250d90cf6a6a6964
Reviewed-on: https://go-review.googlesource.com/c/go/+/770540
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: Nicholas Husin <nsh@golang.org>
Auto-Submit: Damien Neil <dneil@google.com>
This commit is contained in:
Damien Neil 2026-04-24 09:01:39 -07:00 • committed by Gopher Robot
parent 8042aaf03c
commit ea0da4047c
2 changed files with 52 additions and 37 deletions
Download patch file Download diff file Expand all files Collapse all files

78
src/net/http/internal/http2/transport.go
View file

@ -727,41 +727,6 @@ func (cc *ClientConn) SetDoNotReuse() {
cc.doNotReuse = true
}
func (cc *ClientConn) setGoAway(f *GoAwayFrame) {
cc.mu.Lock()
defer cc.mu.Unlock()
old := cc.goAway
cc.goAway = f
// Merge the previous and current GoAway error frames.
if cc.goAwayDebug == "" {
cc.goAwayDebug = string(f.DebugData())
}
if old != nil && old.ErrCode != ErrCodeNo {
cc.goAway.ErrCode = old.ErrCode
}
last := f.LastStreamID
for streamID, cs := range cc.streams {
if streamID <= last {
// The server's GOAWAY indicates that it received this stream.
// It will either finish processing it, or close the connection
// without doing so. Either way, leave the stream alone for now.
continue
}
if streamID == 1 && cc.goAway.ErrCode != ErrCodeNo {
// Don't retry the first stream on a connection if we get a non-NO error.
// If the server is sending an error on a new connection,
// retrying the request on a new one probably isn't going to work.
cs.abortStreamLocked(fmt.Errorf("http2: Transport received GOAWAY from server ErrCode:%v", cc.goAway.ErrCode))
} else {
// Aborting the stream with errClentConnGotGoAway indicates that
// the request should be retried on a new connection.
cs.abortStreamLocked(errClientConnGotGoAway)
}
}
}
// CanTakeNewRequest reports whether the connection can take a new request,
// meaning it has not been closed or received or sent a GOAWAY.
//
@ -2067,6 +2032,9 @@ func (cc *ClientConn) countReadFrameError(err error) {
f("read_frame_other")
}
// errStopReadLoop is an error which cleanly exits the client's read loop.
var errStopReadLoop = errors.New("client connection is closing (BUG: this is not user visible)")
func (rl *clientConnReadLoop) run() error {
cc := rl.cc
gotSettings := false
@ -2127,7 +2095,7 @@ func (rl *clientConnReadLoop) run() error {
cc.logf("Transport: unhandled response frame type %T", f)
}
if err != nil {
if VerboseLogs {
if VerboseLogs && err != errStopReadLoop {
cc.vlogf("http2: Transport conn %p received error from processing frame %v: %v", cc, summarizeFrame(f), err)
}
return err
@ -2647,7 +2615,43 @@ func (rl *clientConnReadLoop) processGoAway(f *GoAwayFrame) error {
fn("recv_goaway_" + f.ErrCode.stringToken())
}
}
cc.setGoAway(f)
cc.mu.Lock()
defer cc.mu.Unlock()
old := cc.goAway
cc.goAway = f
// Merge the previous and current GoAway error frames.
if cc.goAwayDebug == "" {
cc.goAwayDebug = string(f.DebugData())
}
if old != nil && old.ErrCode != ErrCodeNo {
cc.goAway.ErrCode = old.ErrCode
}
last := f.LastStreamID
if len(cc.streams) == 0 {
// Received a GOAWAY and no streams active, just close the conn.
return errStopReadLoop
}
for streamID, cs := range cc.streams {
if streamID <= last {
// The server's GOAWAY indicates that it received this stream.
// It will either finish processing it, or close the connection
// without doing so. Either way, leave the stream alone for now.
continue
}
if streamID == 1 && cc.goAway.ErrCode != ErrCodeNo {
// Don't retry the first stream on a connection if we get a non-NO error.
// If the server is sending an error on a new connection,
// retrying the request on a new one probably isn't going to work.
cs.abortStreamLocked(fmt.Errorf("http2: Transport received GOAWAY from server ErrCode:%v", cc.goAway.ErrCode))
} else {
// Aborting the stream with errClentConnGotGoAway indicates that
// the request should be retried on a new connection.
cs.abortStreamLocked(errClientConnGotGoAway)
}
}
return nil
}

11
src/net/http/internal/http2/transport_test.go
View file

@ -2215,6 +2215,17 @@ func testTransportUsesGoAwayDebugError(t testing.TB, failMidBody bool) {
}
}
// https://go.dev/issue/68440 -- receiving a GoAway when there are no outstanding requests
// should immediately close the connection.
func TestTransportGoAwayWithNoConns(t *testing.T) { synctestTest(t, testTransportGoAwayWithNoConns) }
func testTransportGoAwayWithNoConns(t testing.TB) {
tt := newTestTransportWithUnusedConn(t)
tc := tt.getConn()
tc.greet()
tc.writeGoAway(1, ErrCodeNo, nil)
tc.wantClosed()
}
func testTransportReturnsUnusedFlowControl(t testing.TB, oneDataFrame bool) {
tc := newTestClientConn(t)
tc.greet()