-
Notifications
You must be signed in to change notification settings - Fork 6.2k
8283555: G1: Concurrent mark accesses uninitialized BOT of closed archive regions #7923
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
8283555: G1: Concurrent mark accesses uninitialized BOT of closed archive regions #7923
Conversation
|
👋 Welcome back tschatzl! A progress list of the required criteria for merging this PR into |
|
/label add hotspot-gc |
|
@tschatzl |
Webrevs
|
src/hotspot/share/cds/filemap.cpp
Outdated
| "Null closed_heap_regions array with non-zero count"); | ||
| G1CollectedHeap::heap()->fill_archive_regions(closed_heap_regions, | ||
| num_closed_heap_regions); | ||
| // G1 marking uses the BOT for object chunking; for this reason we need to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to have a reference to the actual method/class name performing "object chunking" in the comment.
|
@tschatzl This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be: You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 1 new commit pushed to the
Please see this link for an up-to-date comparison between the source branch of this pull request and the ➡️ To integrate this PR with the above commit message to the |
walulyai
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm!
|
Thanks @albertnetymk @walulyai for your reviews |
|
Going to push as commit 19f01ab.
Your commit was automatically rebased without conflicts. |
Hi all,
please review this change that fixes crashes occurring in the Loom repo because of missing initialization in the BOT of closed archive regions. This is a bug here too, just typically not occurring.
So during marking through the closed archive regions (for statistics mainly) it can happen that there are objects in these closed archive regions that are split into chunks. To get the start of the object that contains this chunk, we use the BOT.
Since it is not initialized for the closed archive regions G1 crashes as that call returns garbage.
The fix is to not only initialize open archive regions, but also closed archive regions.
The problem started with JDK-8264908 which means that there will be a JDK 18 backport.
Testing: failing test with Loom, GHA
Thanks,
Thomas
Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/7923/head:pull/7923$ git checkout pull/7923Update a local copy of the PR:
$ git checkout pull/7923$ git pull https://git.openjdk.java.net/jdk pull/7923/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 7923View PR using the GUI difftool:
$ git pr show -t 7923Using diff file
Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/7923.diff