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

Conor Cleary ccleary at openjdk.org
Tue Oct 18 08:56:09 UTC 2022


On Fri, 7 Oct 2022 17:19:14 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

>> Conor Cleary has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - 8288717: Updated Test Summary
>>  - 8288717: Updated property name, test format and logs
>
> 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.

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

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


More information about the net-dev mailing list