RFR: 8223696: java/net/httpclient/MaxStreams.java failed with didn't finish within the time-out

[Daniel Jeliński]

On Tue, 23 Jan 2024 14:46:54 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

>> The test was occasionally failing with TestNG timeout. When a TestNG test times out, TestNG interrupts the test thread, and starts another test case in a new thread. The timeout is invisible to JTReg, and the timeout failure handlers are not run.
>> 
>> This PR removes the TestNG timeout. It also removes `-ea -esa` from the test command line (the test does not use `assert`), and reduces the amount of synchronization objects used.
>> 
>> I verified that the timeouts are now handled by JTReg. The test still passes.
>
> test/jdk/java/net/httpclient/MaxStreams.java line 208:
> 
>> 206: 
>> 207:                 is.readAllBytes();
>> 208:                 int c = counter.getAndIncrement();
> 
> That looks mostly fine, but it leaves a race condition where `counter.set(0)` could en up being executed after the next call to the test method happens. I believe that's what the `canStartTestRun` semaphore was trying to prevent. Instead of trying to acquire the semaphore at the beginning of the next test run, maybe we should use it (or some other primitive) to make sure that the current test method doesn't finish before `counter.set(0)`  has been executed?

I moved the counter.set line before sendResponseHeaders. This should take care of the necessary synchronization: the server sets the counter to zero before sending the response, the response needs to be received before the test completes, the test needs to complete before another one is started.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17536#discussion_r1463432958