RFR: 8288717: Add a means to close idle connections in HTTP/2 connection pool [v2]

[Conor Cleary]

On Tue, 18 Oct 2022 08:53:52 GMT, Conor Cleary <ccleary at openjdk.org> wrote:

>> src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java line 730:
>> 
>>> 728:             if (idleConnectionTimeoutEvent != null) {
>>> 729:                 Log.logTrace("idleConnectionTimeout timeout fired, shutting down connection: {0}", t.getMessage());
>>> 730:             } else if (!(t instanceof EOFException) || isActive()) {
>> 
>> That doesn't seem right to me. If `idleConnectionTimeoutEvent` is not null it means that we have armed the timeout, it doesn't mean that it has fired. We're not shutting down the connection, the connection is simply idle, and thus we should log exceptions.
>
> Good point, the setup here seems that an idleConnectionTimeoutEvent being not null could cause its log to be incorrectly logged (like you said, it doesn't mean it has fired.
> 
> I'm thinking maybe here I could include an additional condition in the form of a check to see if the idleConnectionTimeoutEvent has fired or not as well as the null check.

With the latest changes, I'm proposing the additional check for the timeout firing as well as the null check. Also logging with logError as supposed to using logTrace as the idleConnectionTimeoutEvent produceces a ConnectTimeoutException

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

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