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

[Paul Hohensee]

On Thu, 14 Aug 2025 13:14:47 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.
>> 
>> 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".
>> 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 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 refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains two new commits since the last revision:
> 
>  - Fix build and test
>  - Backport bd6152f5967107d7b32db9bcfa224fc07314f098

Marked as reviewed by phh (Reviewer).

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

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