On Wed, 12 Nov 2025 10:47:30 GMT, Volkan Yazici <[email protected]> wrote:
>> Jaikiran Pai has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - mark jdk.internal.net.http.Http2Connection as Closable
>> - reduce number of concurrent requests
>
> test/jdk/java/net/httpclient/http2/BurstyRequestsTest.java line 102:
>
>> 100: private static Field openConnections; // Set<>
>> jdk.internal.net.http.HttpClientImpl#openedConnections
>> 101:
>> 102: private static SSLContext sslContext;
>
> Is SSL a necessity for this test? Put another way, does SSL play a role in
> the connection leakage?
I've now updated the PR to use regular HTTP/2 server without TLS. TLS doesn't
play a role in this issue.
> test/jdk/java/net/httpclient/http2/BurstyRequestsTest.java line 195:
>
>> 193: // using reflection, return the
>> jdk.internal.net.http.HttpClientImpl instance held
>> 194: // by the given client
>> 195: private static Object reflectHttpClientImplInstance(final
>> HttpClient client) throws Exception {
>
> Instead, you can use `Http3ConnectionAccess::impl` too.
Hello Volkan, I wasn't aware of this test class and it looks like it has been
minimally used. The `impl()` method on that test class is a package private
method which then means that I will have to include a `package` statement to
this new test itself. Or change the `impl()` method to `public`. For now I
decided to not fiddle with it. If you would like me to pursue it further in
this PR, let me know and I can do that.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28233#discussion_r2541325409
PR Review Comment: https://git.openjdk.org/jdk/pull/28233#discussion_r2541323780
