On Fri, 4 Apr 2025 14:55:45 GMT, Daniel Fuchs <[email protected]> wrote:
>> 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...
Yes, I think othervm would be fine. In fact, I was adding a review comment in
another context, where I suspect we might be forced to use the othervm mode
anyway.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24154#discussion_r2028992574
