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
8252871: fatal error: must own lock JvmtiThreadState_lock #60
Conversation
👋 Welcome back rehn! A progress list of the required criteria for merging this PR into |
@robehn The following labels will be automatically applied to this pull request: When this pull request is ready to be reviewed, an RFR email will be sent to the corresponding mailing lists. If you would like to change these labels, use the |
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.
Thanks for catching up this. Looks good.
@robehn This change now passes all automated pre-integration checks. In addition to the automated checks, the change must also fulfill all project specific requirements After integration, the commit message will be:
Since the source branch of this PR was last updated there have been 13 commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid automatic rebasing, please merge ➡️ To integrate this PR with the above commit message to the |
Mailing list message from David Holmes on hotspot-dev: Hi Robbin, On 8/09/2020 4:56 am, Robbin Ehn wrote:
Ouch! We continue to get bitten by the fact the executor of a handshake
That avoids the problem but it is not really a sufficient check - we More importantly this issue shows that the locking code is in fact If a handshake operation can be executed by either the target thread or
There is already a bug for that: https://bugs.openjdk.java.net/browse/JDK-8252816 so if this proceeds you should add that issue to the PR. Thanks, |
Hi David,
We could add this. But I would prefer not doing it in this bug fix.
Right now we know that because I traced the code :) As I said I can make the requesting thread known to execution thread,
A problem is that we use a safepoiting global lock to protect per thread resource. So we already have proxied locks and in other cases just avoiding them (by just checking if at safepoint). And as I said a few times now, I said I can make the requesting thread known to execution thread,
The problem is that it's a safepointing lock.
Ok, I'll do that. Thanks for having a look, let me know howto proceed.
|
/issue add JDK-8252816 |
@robehn |
Mailing list message from Robbin Ehn on hotspot-dev: Hi David, our mail service did the wrong thing here. This was a reply to your comment, so you should have been in To-field, Thanks, Robbin On 2020-09-08 09:39, Robbin Ehn wrote: |
Mailing list message from David Holmes on hotspot-dev: Hi Robbin, On 8/09/2020 5:39 pm, Robbin Ehn wrote:
Okay we can put in the current proposed fix to get the test working
I'm sure you get my point though. :)
Okay ... we know that the VMThread and safepoint VMops are special, and
Understood. I have a general concern with locking in relation to handshakes, that we So if we cannot simply grab the necessary locks as part of the handshake Thanks, |
Hi David,
I created https://bugs.openjdk.java.net/browse/JDK-8252902 Thanks, Robbin
|
/integrate |
@robehn Since your change was applied there have been 13 commits pushed to the
Your commit was automatically rebased without conflicts. Pushed as commit 704f784. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
We should use interface as_virtual/as_escaped after alias refactor. Co-authored-by: Xin Liu <xxinliu@amazon.com>
When these two methods (set_frame_pop/clear_frame_pop) are called in a handshake the requesting thread will have lock the JvmtiThreadState_lock.
But the thread executing one of these in the handshake may not be the owner.
So we only check that JvmtiThreadState_lock is locked.
When verifying the callers to these methods I notice "clear_to_frame_pop" was unused, so instead of fixing it I remove it.
Passes testing locally, still running T3 and T7.
Now passed!
Progress
Issues
Reviewers
Download
$ git fetch https://git.openjdk.java.net/jdk pull/60/head:pull/60
$ git checkout pull/60