RFR: 8352066: JVM.commit() and JVM.flush() exhibit race conditions against JFR epochs
Greetings, This change set removes some race conditions involving JFR epochs. For details, please see the JIRA issue. Testing: jdk_jfr, TestChunkIntegrity testing Thanks Markus ------------- Commit messages: - 8352066 Changes: https://git.openjdk.org/jdk/pull/24061/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=24061&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8352066 Stats: 64 lines in 9 files changed: 5 ins; 36 del; 23 mod Patch: https://git.openjdk.org/jdk/pull/24061.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/24061/head:pull/24061 PR: https://git.openjdk.org/jdk/pull/24061
On Fri, 14 Mar 2025 14:55:40 GMT, Markus Grönlund <mgronlun@openjdk.org> wrote:
Greetings,
This change set removes some race conditions involving JFR epochs. For details, please see the JIRA issue.
Testing: jdk_jfr, TestChunkIntegrity testing
Thanks Markus
I noticed that a boolean flag was added to the JfrCheckpointManager::on_rotation, but couldn't see where it was being used. Was this by design or a mistake? ------------- PR Comment: https://git.openjdk.org/jdk/pull/24061#issuecomment-2725210691
On Fri, 14 Mar 2025 14:55:40 GMT, Markus Grönlund <mgronlun@openjdk.org> wrote:
Greetings,
This change set removes some race conditions involving JFR epochs. For details, please see the JIRA issue.
Testing: jdk_jfr, TestChunkIntegrity testing
Thanks Markus
I noticed a boolean flag was added to the JfrCheckpointManager::on_rotation, but I couldn't see where it was used. Was this by design or a mistake?
Good, you noticed, and I forgot to describe this. I am adding the notify_threads() call also for Safepoint::clear(), not only for Safepoint::write(), to ensure ongoing commits are also aborted when a new physical recording starts. Should a thread, for example, get severely unscheduled after, let's say, returning from, or get stuck in the VM for a more extended period during a commit. It seems very unlikely to be necessary now that I am writing this, but it's another "clearing" activity when a physical recording begins. I don't have tangible proof that is not necessary yet. If we find out it's not needed later, we can remove it. ------------- PR Comment: https://git.openjdk.org/jdk/pull/24061#issuecomment-2725386494
Greetings,
This change set removes some race conditions involving JFR epochs. For details, please see the JIRA issue.
Testing: jdk_jfr, TestChunkIntegrity testing
Thanks Markus
Markus Grönlund has updated the pull request incrementally with one additional commit since the last revision: restore checkpoint manager on_rotation ------------- Changes: - all: https://git.openjdk.org/jdk/pull/24061/files - new: https://git.openjdk.org/jdk/pull/24061/files/2f4d1827..349045dc Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=24061&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=24061&range=00-01 Stats: 5 lines in 2 files changed: 0 ins; 2 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/24061.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/24061/head:pull/24061 PR: https://git.openjdk.org/jdk/pull/24061
On Fri, 14 Mar 2025 18:35:06 GMT, Markus Grönlund <mgronlun@openjdk.org> wrote:
Greetings,
This change set removes some race conditions involving JFR epochs. For details, please see the JIRA issue.
Testing: jdk_jfr, TestChunkIntegrity testing
Thanks Markus
Markus Grönlund has updated the pull request incrementally with one additional commit since the last revision:
restore checkpoint manager on_rotation
Marked as reviewed by egahlin (Reviewer). ------------- PR Review: https://git.openjdk.org/jdk/pull/24061#pullrequestreview-2686490467
On Fri, 14 Mar 2025 14:55:40 GMT, Markus Grönlund <mgronlun@openjdk.org> wrote:
Greetings,
This change set removes some race conditions involving JFR epochs. For details, please see the JIRA issue.
Testing: jdk_jfr, TestChunkIntegrity testing
Thanks Markus
I noticed that a boolean flag was added to the JfrCheckpointManager::on_rotation, but couldn't see where it was being used. Was this by design or a mistake?
These are remnants from a previous iteration. Thanks for noticing. I will remove it. ------------- PR Comment: https://git.openjdk.org/jdk/pull/24061#issuecomment-2725405512
On Fri, 14 Mar 2025 14:55:40 GMT, Markus Grönlund <mgronlun@openjdk.org> wrote:
Greetings,
This change set removes some race conditions involving JFR epochs. For details, please see the JIRA issue.
Testing: jdk_jfr, TestChunkIntegrity testing
Thanks Markus
This pull request has now been integrated. Changeset: d207ed3f Author: Markus Grönlund <mgronlun@openjdk.org> URL: https://git.openjdk.org/jdk/commit/d207ed3f7cb810e3c0c8a8cd4d9aaa65164c6d16 Stats: 59 lines in 8 files changed: 3 ins; 36 del; 20 mod 8352066: JVM.commit() and JVM.flush() exhibit race conditions against JFR epochs Reviewed-by: egahlin ------------- PR: https://git.openjdk.org/jdk/pull/24061
participants (2)
-
Erik Gahlin
-
Markus Grönlund