[jdk17u-dev] RFR: 8343855: HTTP/2 ConnectionWindowUpdateSender may miss some unprocessed DataFrames from closed streams [v3]

[Ralf Schmelter]

On Fri, 15 Aug 2025 09:50:30 GMT, Goetz Lindenmaier <goetz at openjdk.org> wrote:

>> I backport this for parity with 17.0.17-oracle.
>> Also, it is a follow-up to [JDK-8342075: HttpClient: improve HTTP/2 flow control checks](https://bugs.openjdk.org/browse/JDK-8342075).
>> 
>> It needed resolves and adaptions:
>> 
>> **Resolved** the first chunk in Stream.java which adds the declaration of inputQLock.
>> The declaraions in the context have been modified by 
>> [8308310: HttpClient: Avoid logging or locking from within synchronized blocks](https://bugs.openjdk.org/browse/JDK-8308310).
>> Omitted one chunk fixing white space.
>> See first commit.
>> 
>> **Adaptions** can be found in the second commit.
>> 
>> The change uses locks from 8308310 and introduces a new lock.
>> To make this work further adaptions are needed (see extra commit):
>> I add edimports required by the new lock inputQLock.
>> 
>> stateLock is used by this change, but only introduced by JDK-8308310.
>> Where 21 uses stateLock, the implementation in 17 locks
>> itself ("this").  I changed the use of stateLock introduced
>> by this change in pushDateaFrame() accordingly.
>> 
>> Actually, 17 declares a row of methods as synchronized
>> that are not handled by stateLock in 21, e.g. isCanceled().
>> So there is a difference in behavior wrt. this lock
>> between 17 and 21. 
>> 
>> I needed to fix two build issues in StreamFlowControlTest.java:
>> 
>> .../test/jdk/java/net/httpclient/http2/StreamFlowControlTest.java:299: error: incompatible types: try-with-resources not applicable to variable type
>>         try (var client = HttpClient.newBuilder().sslContext(sslContext).build()) {
>>                  ^
>>     (HttpClient cannot be converted to AutoCloseable)
>> 
>> /.../test/jdk/java/net/httpclient/http2/StreamFlowControlTest.java:300: error: cannot find symbol
>>             var request = HttpRequest.newBuilder(URI.create(h2Head)).HEAD().build();
>>                                                                     ^
>>   symbol:   method HEAD()
>>   location: interface Builder
>> 
>> The first I resolved similar to other places in this test in the backport that
>> brought the test to 17: https://github.com/openjdk/jdk17u-dev/pull/3786/commits/073bb3d5b0391cf9ad419a9b2be0d30afba1427b
>> Thanks to Aleksey for this detail.
>> 
>> For the second issue I replaced HEAD() by it's implementation.
>
> Goetz Lindenmaier 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 three additional commits since the last revision:
> 
>  - Merge branch 'master' into goetz_backport_8343855
>  - Fix build and test
>  - Backport bd6152f5967107d7b32db9bcfa224fc07314f098

Looks good.

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

Marked as reviewed by rschmelter (Reviewer).

PR Review: https://git.openjdk.org/jdk17u-dev/pull/3852#pullrequestreview-3123803904