8371046: Segfault in compiler/whitebox/StressNMethodRelocation.java with -XX:+UseZGC #28241
+32
−25
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
JDK-8371046
This pull request fixes two crashes (see below) and adds
InvalidationReason::RELOCATEDto better describe why an nmethod is marked not entrant during relocation.1. Test Bug
It’s possible for an
nmethodto be unloaded without its_statebeing explicitly set tonot_entrant. Checking onlyis_in_use()isn’t sufficient, since thenmethodmay already be in the process of unloading and therefore may not have a lock (as with ZGC, wherenmethodsare locked individually).The fix adds an additional
is_unloading()check in WhiteBox before acquiring the lock.This issue was reproducible fairly consistently (every few runs) by executing
compiler/whitebox/StressNMethodRelocation.javawith-XX:+UseZGC -XX:ReservedCodeCacheSize=32mAfter applying this patch, the original crash stopped occurring, though a more infrequent crash was still observed.
2. Implementation Bug
nmethod::relocateworks by copying the instructions of annmethodand then adjusting the call sites to account for new PC-relative offsets.Previously, this fix-up happened after calling
post_init(), which registers thenmethodand makes it visible to the GC. This introduced a race condition where the GC might attempt to resolve a call site before it had been fixed.The fix ensures that all call sites are patched before the
nmethodis registered.In testing, the crash previously occurred roughly 60 times in 5,000 runs (~1.2%). With this patch, no crashes were observed in the same number of runs.
Progress
Issue
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/28241/head:pull/28241$ git checkout pull/28241Update a local copy of the PR:
$ git checkout pull/28241$ git pull https://git.openjdk.org/jdk.git pull/28241/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 28241View PR using the GUI difftool:
$ git pr show -t 28241Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/28241.diff
Using Webrev
Link to Webrev Comment