On Tue, 11 Jun 2024 13:09:21 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
> Can I please get a review of this test-only change which proposes to address > intermittent failures in > `test/jdk/java/io/Serializable/concurrentClassDescLookup/ConcurrentClassDescLookup.java`? > > This addresses https://bugs.openjdk.org/browse/JDK-8222884. As noted in that > issue, the test in its current form has a potential race. The test main > thread launches 50 "successful lookup" tasks and 50 "failed lookup" tasks. > Each task is a `Thread`. The test passes an `Object` instance shared by 50 of > these tasks and each task when it starts execution in its `run()` method does > a `wait()` on that `Object` instance. The test main thread then after > launching 50 tasks, sleeps for a second (to let the 50 `Thread`s start > execution and arrive at the `Object.wait()`) and then after waking up the > main thread does a `notifyAll()` on that `Object` instance. In theory and > likely in practice too, it's possible that (especially) for the last batch of > 50 tasks (the "failing lookup" tasks), the `notifyAll()` might end up getting > invoked before one or more task Thread(s) have actually done a `wait()`. That > can then mean that one or more task `Thread`(s) will keep `wait()`ing > indefinitely with no on e `notify()`ing the `Object` instance to proceed with the task execution. > > The commit in this PR removes the use of `Object.wait()/notifyAll()` and > instead uses a `CountdownLatch` to allow for a more deterministic barrier. > The test continues to pass with this change. Additionally, this change was > run against a CI setup, by Matthias, where it was previously failing > intermittently. Matthias reports that it hasn't failed after this change. LGTM ------------- Marked as reviewed by rriggs (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19659#pullrequestreview-2110524082
