Re: [jdk23] RFR: 8222884: ConcurrentClassDescLookup.java times out intermittently
On Mon, 24 Jun 2024 09:40:14 GMT, Christoph Langer wrote: > Hi all, > > This pull request contains a backport of > [JDK-8222884](https://bugs.openjdk.org/browse/JDK-8222884), commit > [bd046d9b](https://github.com/openjdk/jdk/commit/bd046d9b9e79e4eea89c72af358961ef6e98e660) > from the [openjdk/jdk](https://git.openjdk.org/jdk) repository. > > The commit being backported was authored by Jaikiran Pai on 12 Jun 2024 and > was reviewed by Roger Riggs and Matthias Baesken. > > Thanks! LGTM. - Marked as reviewed by mdoerr (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19853#pullrequestreview-2135601316
[jdk23] RFR: 8222884: ConcurrentClassDescLookup.java times out intermittently
Hi all, This pull request contains a backport of [JDK-8222884](https://bugs.openjdk.org/browse/JDK-8222884), commit [bd046d9b](https://github.com/openjdk/jdk/commit/bd046d9b9e79e4eea89c72af358961ef6e98e660) from the [openjdk/jdk](https://git.openjdk.org/jdk) repository. The commit being backported was authored by Jaikiran Pai on 12 Jun 2024 and was reviewed by Roger Riggs and Matthias Baesken. Thanks! - Commit messages: - Backport bd046d9b9e79e4eea89c72af358961ef6e98e660 Changes: https://git.openjdk.org/jdk/pull/19853/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19853&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8222884 Stats: 36 lines in 1 file changed: 7 ins; 7 del; 22 mod Patch: https://git.openjdk.org/jdk/pull/19853.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19853/head:pull/19853 PR: https://git.openjdk.org/jdk/pull/19853
Re: RFR: 8222884: ConcurrentClassDescLookup.java times out intermittently
On Tue, 11 Jun 2024 13:09:21 GMT, Jaikiran Pai 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. Thank you Roger and Matthias for the reviews. - PR Comment: https://git.openjdk.org/jdk/pull/19659#issuecomment-2162143042
Re: RFR: 8222884: ConcurrentClassDescLookup.java times out intermittently
On Tue, 11 Jun 2024 13:09:21 GMT, Jaikiran Pai 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. Marked as reviewed by mbaesken (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/19659#pullrequestreview-2110799797
Re: RFR: 8222884: ConcurrentClassDescLookup.java times out intermittently
On Tue, 11 Jun 2024 13:09:21 GMT, Jaikiran Pai 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
RFR: 8222884: ConcurrentClassDescLookup.java times out intermittently
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 one `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. - Commit messages: - 8222884: ConcurrentClassDescLookup.java times out intermittently Changes: https://git.openjdk.org/jdk/pull/19659/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19659&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8222884 Stats: 36 lines in 1 file changed: 7 ins; 7 del; 22 mod Patch: https://git.openjdk.org/jdk/pull/19659.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19659/head:pull/19659 PR: https://git.openjdk.org/jdk/pull/19659
