Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

transport: prevent deadlock in transport Close when GoAway write hangs #7662

Merged
merged 3 commits into from
Oct 8, 2024
Merged

transport: prevent deadlock in transport Close when GoAway write hangs #7662

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5992ffa
transport/http2_client: fixed flaky test Test/ClientCloseReturnsEarly…
aranjans Sep 23, 2024
cddd130
minimised acquiring t.mu lock
aranjans Oct 3, 2024
4eaf6ab
Unlock t.mu after t.goAwayDebugMessage
aranjans Oct 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions internal/transport/http2_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -527,8 +527,9 @@ func (t *http2Client) getPeer() *peer.Peer {
// to be the last frame loopy writes to the transport.
func (t *http2Client) outgoingGoAwayHandler(g *goAway) (bool, error) {
t.mu.Lock()
defer t.mu.Unlock()
if err := t.framer.fr.WriteGoAway(t.nextID-2, http2.ErrCodeNo, g.debugData); err != nil {
maxStreamID := t.nextID - 2
t.mu.Unlock()
if err := t.framer.fr.WriteGoAway(maxStreamID, http2.ErrCodeNo, g.debugData); err != nil {
return false, err
}
return false, g.closeConn
Expand Down Expand Up @@ -1008,6 +1009,9 @@ func (t *http2Client) Close(err error) {
// should unblock it so that the goroutine eventually exits.
t.kpDormancyCond.Signal()
}
// Append info about previous goaways if there were any, since this may be important
// for understanding the root cause for this connection to be closed.
goAwayDebugMessage := t.goAwayDebugMessage
t.mu.Unlock()

// Per HTTP/2 spec, a GOAWAY frame must be sent before closing the
Expand All @@ -1026,10 +1030,6 @@ func (t *http2Client) Close(err error) {
t.cancel()
t.conn.Close()
channelz.RemoveEntry(t.channelz.ID)
// Append info about previous goaways if there were any, since this may be important
// for understanding the root cause for this connection to be closed.
_, goAwayDebugMessage := t.GetGoAwayReason()

var st *status.Status
if len(goAwayDebugMessage) > 0 {
st = status.Newf(codes.Unavailable, "closing transport due to: %v, received prior goaway: %v", err, goAwayDebugMessage)
Expand Down