RFR: 8223696: java/net/httpclient/MaxStreams.java failed with didn't finish within the time-out
Daniel Fuchs
dfuchs at openjdk.org
Tue Jan 23 14:49:31 UTC 2024
On Tue, 23 Jan 2024 13:15:46 GMT, Daniel Jeliński <djelinski 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.
Changes requested by dfuchs (Reviewer).
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?
-------------
PR Review: https://git.openjdk.org/jdk/pull/17536#pullrequestreview-1838974313
PR Review Comment: https://git.openjdk.org/jdk/pull/17536#discussion_r1463392445
More information about the net-dev
mailing list