RFR: 8350279: HttpClient: Add a new HttpResponse method to identify connections [v6]

Jaikiran Pai jpai at openjdk.org
Fri Apr 4 14:51:08 UTC 2025


On Fri, 4 Apr 2025 12:47:19 GMT, Volkan Yazici <vyazici at openjdk.org> wrote:

>> Adds `HttpResponse::connectionLabel` method that provides an identifier for the connection.
>> 
>> **Implementation note:** The feature is facilitated by `HttpConnection::label`, which should not be confused with `HttpConnection::id`. This distinction is explained in the JavaDoc of both properties.
>
> Volkan Yazici has updated the pull request incrementally with five additional commits since the last revision:
> 
>  - Remove timeout from `CountDownLatch::await` calls
>  - Replace `@AutoClose` with a corresponding `@AfterEach` method
>  - Remove IDE-specific `OptionalGetWithoutIsPresent` warning suppression
>  - Improve `HttpConnection::label` JavaDoc
>  - Start from 1 while labeling connections

test/jdk/java/net/httpclient/HttpResponseConnectionLabelTest.java line 190:

> 188:         private static HttpTestServer createServer(Version version, SSLContext sslContext) {
> 189:             try {
> 190:                 return HttpTestServer.create(version, sslContext, ForkJoinPool.commonPool());

The HTTP/2 test server in the JDK shuts down the `Executor` that is passed to that server (like this one), when the server is stopped. I had a look at the specification and implementation of `ForkJoinPool.commonPool()` - it is specified to not close/shutdown the common pool. So I think it is OK to use the `commonPool()` as the `Executor` for these servers.

Although, since this test is agentvm test, we never known what state the commonPool() executor is in due to some previously run test(s). Maybe we should consider having a test specific Executor in this test, like we do in some other tests within the httpclient tests.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24154#discussion_r2028968733


More information about the net-dev mailing list