RFR: 8309118: HttpClient: Add more tests for 100 ExpectContinue with HTTP/2 [v3]
Conor Cleary
ccleary at openjdk.org
Wed Oct 18 14:51:10 UTC 2023
On Mon, 9 Oct 2023 13:25:58 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:
>> Conor Cleary has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains ten additional commits since the last revision:
>>
>> - 8309118: Fixed error in handleReset ReentrantLock
>> - 8309118: Updates to Stream cancelImpl
>> - 8309118: Refactored test, updated incoming_reset
>> - Merge branch 'master' into JDK-8309118
>> - 8309118: Cleanup identifiers and whitespace
>> - 8309118: Removed unused try-with-resources
>> - 8309118: Improve comments and annotations
>> - 8309118: Remove local test timeout value
>> - 8309118: Add more tests for 100 ExpectContinue with HTTP/2
>
> src/java.net.http/share/classes/jdk/internal/net/http/Http2ClientImpl.java line 234:
>
>> 232: connections.values().forEach(this::close);
>> 233: connections.remove(entry.getKey(), entry.getValue());
>> 234: }
>
> I'd suggest to change `this::close(Http2Connection)` to return boolean (instead of void) and make it always return`true`, and then use `removeIf` instead of forEach:
>
>
> connections.values().removeIf(this::close);
So effect of your suggested changes is to remove the connection if close returns `true` as supposed to just removing all connections and
This change is not entirely related to this PR. I will revert it and create a seperate issue. Though the suggestion is good and I will apply it to the give fix for that issue.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15664#discussion_r1364020253
More information about the net-dev
mailing list