-
Notifications
You must be signed in to change notification settings - Fork 146
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
8339133: [8u] Profiler crashes at guarantee(is_result_safe || is_in_asgct()): unsafe access to zombie method #568
base: master
Are you sure you want to change the base?
Conversation
👋 Welcome back apangin! A progress list of the required criteria for merging this PR into |
❗ This change is not yet ready to be integrated. |
Webrevs
|
…sgct()): unsafe access to zombie method
@apangin Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. See OpenJDK Developers’ Guide for more information. |
Hi Andrey, why do you submit this as a jdk 8 PR? Why not fix it upstream? Is it not an issue on JDK 11/17/21 etc? |
@tstuefe The issue has been already fixed in upstream in JDK-8329103 and then backported to 21u, 17u and 11u. |
Okay, thank you for the explanation. Up to @gnu-andrew to decide if this is the right approach. Technically, the patch is fine. |
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
|
/approval request Low risk yet important fix for profiling robustness. Affects only AsyncGetCallTrace code. |
@apangin This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration! |
Dear maintainers, is there anything else I need to do to move this PR forward? |
When running AsyncGetCallTrace based profiler, JVM occasionally crashes on the guarantee at codeCache.cpp:287:
According to the stack trace, a thread is inside
AsyncGetCallTrace
at the moment of crash, butis_in_asgct() == false
.Current implementation of
AsyncGetCallTrace
is not reentrant, since it incorrectly resetsin_asgct
flag of the current thread. The fix is similar to JDK-8329103:in_asgct
should be reset to previous value instead offalse
.The fix is trivial and safe: it affects only
AsyncGetCallTrace
.Testing: tier1, renaissance suite with async-profiler enabled
Progress
Issue
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk8u-dev.git pull/568/head:pull/568
$ git checkout pull/568
Update a local copy of the PR:
$ git checkout pull/568
$ git pull https://git.openjdk.org/jdk8u-dev.git pull/568/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 568
View PR using the GUI difftool:
$ git pr show -t 568
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk8u-dev/pull/568.diff
Webrev
Link to Webrev Comment