On Fri, 11 Apr 2025 17:58:58 GMT, Daniel Fuchs <[email protected]> wrote:
>> Volkan Yazici has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Add `@implNote` to state that the built-in is always non-empty
>
> test/jdk/java/net/httpclient/HttpResponseConnectionLabelTest.java line 188:
>
>> 186: private static HttpTestServer createServer(Version version,
>> SSLContext sslContext) {
>> 187: try {
>> 188: return HttpTestServer.create(version, sslContext,
>> ForkJoinPool.commonPool());
>
> OK - we definitely MUST NOT use the common FJP here.
>
> 1. our HTTP/2 servers start their dispatching thread, which blocks on
> accept(), in the executor.
> 2. the tests will block (CountDownLatch) in the exchange, which is a thread
> in the executor.
> 3. the test creates at least two HTTP/2 servers (maybe more) upfront.
>
> If you run this test on a machine that has a low number of processors - say 4
> - then the FJP will maybe have 3 threads at most. You can see where that
> goes...
If no executor is provided the HTTP/2 server will create its own. The HTTP/1.1
server however will run the exchange in the dispacher thread - so you need to
at least provide an executor for the HTTP/1.1 server if you don't want to wedge
when making parallel requests. But not the common FJP ;-)
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24154#discussion_r2040059863
