-
Notifications
You must be signed in to change notification settings - Fork 241
8212155: Race condition when posting dynamic_code_generated event leads to JVM crash #3051
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
base: master
Are you sure you want to change the base?
Conversation
👋 Welcome back krk! A progress list of the required criteria for merging this PR into |
❗ This change is not yet ready to be integrated. |
This backport pull request has now been updated with issue from the original commit. |
Tagging reviewer of the original commit: @sspitsyn |
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.
Makes sense. Please make sure to run the remaining jvmti
tests:
jdk11u-dev/test/hotspot/jtreg/TEST.groups
Lines 462 to 464 in cec1ef0
# JVMTI tests | |
vmTestbase_nsk_jvmti = \ | |
vmTestbase/nsk/jvmti |
jdk11u-dev/test/hotspot/jtreg/TEST.groups
Lines 59 to 60 in cec1ef0
hotspot_serviceability = \ | |
serviceability |
|
|
/approval request Fixes a critical JVM crash from a race condition when posting dynamic_code_generated JVMTI events. The patch required manual conflict resolution in JvmtiExport.cpp because JvmtiThreadState::state_for() was previously refactored in this branch. The new regression test, DynamicCodeGeneratedTest, required the removal of an unsupported jtreg tag and was verified to fail without the patch and pass with it. This fix has been stable in mainline since June 2021. The risk is low, as the fix is a minimal, defensive check. |
Backport of JDK-8212155.
The patch did not apply cleanly due to a merge conflict. The original code
JvmtiThreadState::state_for(JavaThread::current())
had already been refactored toJavaThread::current()->jvmti_thread_state()
in this branch.I have also removed the
@requires vm.jvmti
tag from the test, as it is not supported by the jtreg version in JDK 11.The
DynamicCodeGeneratedTest
was confirmed to fail without this patch and passes with it.Progress
Issue
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk11u-dev.git pull/3051/head:pull/3051
$ git checkout pull/3051
Update a local copy of the PR:
$ git checkout pull/3051
$ git pull https://git.openjdk.org/jdk11u-dev.git pull/3051/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 3051
View PR using the GUI difftool:
$ git pr show -t 3051
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk11u-dev/pull/3051.diff
Using Webrev
Link to Webrev Comment