On Fri, 7 Oct 2022 17:19:14 GMT, Daniel Fuchs <[email protected]> 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
