On Tue, 23 Jan 2024 13:15:46 GMT, Daniel Jeliński <[email protected]> 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
