Add H2 session level inactivity timer

[shinrich]

The current implementation relies on TCP/TLS level netvc inactivity timer. However, the HTTP/2 protocol allows for many frame types and we may not want all of the possible frame times sent by the client to reset the inactivity timer.

For example, while debugging a Http2ClientSession leak (using PR #3713), I saw cases of clients sending Ping frames and pushing the life of the Http2Client session far beyond the inactivity time beyond the last seen HTTP/2 stream.

This PR adds Http2ClientSession inactivity support much like the Http2Stream inactivity logic. It does not reset the inactivity timer when receiving a Ping frame, so a pinging client cannot pin open a Http2ClientSession.

I think it is reasonable to not include Ping's with extending the activity of a session. But I'd like to hear other peoples opinions on this.

[maskit]

The PING frame (type=0x6) is a mechanism for measuring a minimal
round-trip time from the sender, as well as determining whether an
idle connection is still functional.

However the spec doesn't say "it's for keeping a session active/alive", I'm not so sure we should do this. Clients may use it for keep-alive.

Isn't that situation managed by active timeout?

[masaori335]

Why you don't want allow client keeping HTTP/2 connection alive by PING frame? Do you worry about DoS attack?

[shinrich]

Yes, worried about DoS. Our leaking environment shouldn't really be using HTTP/2 in my opinion, but that is not my choice. It is making single POST requests per session. So in that environment, the session should shut down ASAP. But you are correct, and active timeout should handle that case, and the stream operation is pretty small so we could make the active timeout pretty short. Will give that a try.

[SolidWallOfCode]

If the spec says PING is for "determining whether an idle connection is still functional" that seems the opposite of a keep alive.

[maskit]

@SolidWallOfCode While I agree with that, I think a client would expect that “Pong” means the connection is still functional and also available for a while. Otherwise the pong is useless. So we shouldn’t close the connection if we sent a pong.

[masaori335]

IMO, active timeout will work in some cases, but it looks not enough when people want keep it long.

One idea is adding proxy.config.http2.keep_alive_no_activity_timeout_in as HTTP/2 session level inactivity timeout like proxy.config.http.keep_alive_no_activity_timeout_in for HTTP/1.1. Yes, this is making timeout configs complicated, but we have use cases.

• -1 : Disable this ( PING frame can extend life of H2 connection )
• 0 : Shutdown H2 connection immediately after all streams are processed.
• x : Keep H2 connection x sec alive after all streams are processed. All frames except PING frame can reset this timeout.

[shinrich]

I did some more searches around the internet, and it seems that most implementations are assuming that a client PING will reset the server's inactive timer. In the case I saw this issue, I was running with the zombie event, so I am not sure how big of a problem this is in my scenarios. Rather than complicating timeout configuration, I'm going to withdraw this PR and update the zombie-event PR to reset when it receives a ping frame.