On Fri, 4 Apr 2025 14:48:16 GMT, Jaikiran Pai <j...@openjdk.org> wrote:

>> 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.

Another possibility is to run in /othervm mode - then we don't care...

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

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

Reply via email to