On Fri, 11 Apr 2025 18:05:00 GMT, Daniel Fuchs <[email protected]> wrote:
>> 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 ;-)
In a0859c7c68614e8c3f531b1d4d9357f3f48cf5ab,
- Used a dedicated executor (!= FJP) only for HTTP/1.1 servers (also documented
the rationale in the sources)
- Added `SystemDiagnosticsCollector` to aid with troubleshooting
Verified that test passes on Windows, Linux, and macOS hosts with 30
repetitions.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24154#discussion_r2043850831
