RFR: 8301255: Http2Connection may send too many GOAWAY frames

[Jaikiran Pai]

On Fri, 27 Jan 2023 18:16:20 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

> The following behavior was observed in a test log:
> 
> DEBUG: [HttpClient-2-SelectorManager] [2s 716ms] Http2Connection(SSLTube(SocketTube(2))) Shutting down h2c (closed=true): java.io.EOFException: HTTP/2 client stopped
> DEBUG: [HttpClient-2-SelectorManager] [2s 716ms] Http2Connection(SSLTube(SocketTube(2))) Close all streams
> DEBUG: [HttpClient-2-SelectorManager] [2s 716ms] Http2Connection(SSLTube(SocketTube(2))) Close all streams
> DEBUG: [HttpClient-2-SelectorManager] [2s 716ms] Http2Connection(SSLTube(SocketTube(2))) sending frame: GOAWAY: length=25, streamid=0, flags=0 Error: Not an error Debugdata: Requested by user
> DEBUG: [HttpClient-2-SelectorManager] [2s 716ms] AsyncSSLConnection(SSLTube(SocketTube(2))) added 34 bytes to the write queue
> DEBUG: [HttpClient-2-SelectorManager] [2s 716ms] AsyncSSLConnection(SSLTube(SocketTube(2))) signalling the publisher of the write queue
> DEBUG: [HttpClient-2-SelectorManager] [2s 716ms] Http2Connection(SSLTube(SocketTube(2))) Shutting down h2c (closed=true): java.io.EOFException: HTTP/2 client stopped
> DEBUG: [HttpClient-2-SelectorManager] [2s 716ms] Http2Connection(SSLTube(SocketTube(2))) Close all streams
> DEBUG: [HttpClient-2-SelectorManager] [2s 716ms] Http2Connection(SSLTube(SocketTube(2))) Close all streams
> DEBUG: [HttpClient-2-SelectorManager] [2s 716ms] Http2Connection(SSLTube(SocketTube(2))) sending frame: GOAWAY: length=25, streamid=0, flags=0 Error: Not an error Debugdata: Requested by user
> DEBUG: [HttpClient-2-SelectorManager] [2s 716ms] AsyncSSLConnection(SSLTube(SocketTube(2))) added 34 bytes to the write queue
> DEBUG: [HttpClient-2-SelectorManager] [2s 716ms] AsyncSSLConnection(SSLTube(SocketTube(2))) signalling the publisher of the write queue
> DEBUG: [HttpClient-2-SelectorManager] [2s 716ms] Http2Connection(SSLTube(SocketTube(2))) Shutting down h2c (closed=true): java.io.EOFException: HTTP/2 client stopped 
> etc...
> 
> What happens here is that attempting to send a GOAWAY frame after some error was detected might cause an error which triggers an attempt to shutdown the connection again and cause a new GOAWAY frame to be sent. This seems to happen when trying to shutdown the client and close HTTP/2 TLS connections gracefully. 
> 
> The fix changes the simple 'closed' boolean into a bit mask that can combined the several closed states of the connection: `Http2Connection::shutdown` has  been called, SHUTDOWN is requested (previously was closed=true), a GOAWAY frame has been sent (the connection is half closed local), a GOAWAY connection has been received (the connection is half closed remote). 
> 
> If the connection is already half-closed locally - a GOAWAY frame has been sent already and there's no need to send a new one.
> 
> This will also better help diagnose the actual state of the connection at the time `Http2Connection::shutdown` is called.

Marked as reviewed by jpai (Reviewer).

-------------

PR: https://git.openjdk.org/jdk/pull/12263