Skip to content

Commit

Permalink
http2: fix over-aggressive ignoring of frames while in "go away" mode
Browse files Browse the repository at this point in the history
https://golang.org/cl/31727 made many of the Server Frame processors
ignore incoming frames if the connection was in shutdown mode.

The idea was that it's pointless to do work if we're about to hang up
on them in 250ms anyway for violating a protocol error.

But as of https://golang.org/cl/32412 (graceful shutdown) we can also
be in "go away" mode for ErrCodeNo, which just means to nicely tell
them to GOAWAY and because they did nothing wrong, we don't hang up in
250ms (the value of which gave the peer time to read the error before
the TCP close), but instead we keep the conn open until it's idle.

The combination of the two CLs made TestTransportAndServerSharedBodyRace_h2
flaky, since it was never seeing RST_STREAM on cancelation of requests,
and then Handlers would block forever.

Updates golang/go#17733 (fixes after bundle into std)

Change-Id: I67491c1d7124bc3cb554f9246ea7683f20b6a4e3
Reviewed-on: https://go-review.googlesource.com/32583
Run-TryBot: Brad Fitzpatrick <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: David Crawshaw <[email protected]>
  • Loading branch information
bradfitz committed Nov 3, 2016
1 parent 6c4ac8b commit 569280f
Showing 1 changed file with 2 additions and 11 deletions.
13 changes: 2 additions & 11 deletions http2/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -1185,7 +1185,7 @@ func (sc *serverConn) processPing(f *PingFrame) error {
// PROTOCOL_ERROR."
return ConnectionError(ErrCodeProtocol)
}
if sc.inGoAway {
if sc.inGoAway && sc.goAwayCode != ErrCodeNo {
return nil
}
sc.writeFrame(FrameWriteRequest{write: writePingAck{f}})
Expand All @@ -1194,9 +1194,6 @@ func (sc *serverConn) processPing(f *PingFrame) error {

func (sc *serverConn) processWindowUpdate(f *WindowUpdateFrame) error {
sc.serveG.check()
if sc.inGoAway {
return nil
}
switch {
case f.StreamID != 0: // stream-level flow control
state, st := sc.state(f.StreamID)
Expand Down Expand Up @@ -1229,9 +1226,6 @@ func (sc *serverConn) processWindowUpdate(f *WindowUpdateFrame) error {

func (sc *serverConn) processResetStream(f *RSTStreamFrame) error {
sc.serveG.check()
if sc.inGoAway {
return nil
}

state, st := sc.state(f.StreamID)
if state == stateIdle {
Expand Down Expand Up @@ -1291,9 +1285,6 @@ func (sc *serverConn) processSettings(f *SettingsFrame) error {
}
return nil
}
if sc.inGoAway {
return nil
}
if err := f.ForeachSetting(sc.processSetting); err != nil {
return err
}
Expand Down Expand Up @@ -1365,7 +1356,7 @@ func (sc *serverConn) processSettingInitialWindowSize(val uint32) error {

func (sc *serverConn) processData(f *DataFrame) error {
sc.serveG.check()
if sc.inGoAway {
if sc.inGoAway && sc.goAwayCode != ErrCodeNo {
return nil
}
data := f.Data()
Expand Down

0 comments on commit 569280f

Please sign in to comment.