-
Notifications
You must be signed in to change notification settings - Fork 6.2k
8373366: HandshakeState should disallow suspend ops for disabler threads #28740
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 sspitsyn! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
Webrevs
|
lmesnik
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.
The fix looks good. However, I think the it makes a sense to add regression test that tries to suspend thread while it processing event callback.
Can you please show where mounting/unmounting is disabled for jvmti events. i was unable to fines this place.
|
/cc serviceability |
|
@lmesnik |
pchilano
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.
Hi Serguei, some comments below, thanks.
Right question, thanks. This issue came from testing of the PR #28407 .
I might incorrectly understand your question. Let's discuss it offline. |
An asynchronous handshake operation (
ThreadSelfSuspensionHandshakeClosure) can be installed when the target thread is not in aMountUnmountDisablerscope. But the target thread can enter such scope by the time the operation is self-processed by the target thread.This is fixed by a small tweak in the function
HandshakeOperation* HandshakeState::get_op_for_self(bool allow_suspend, bool check_async_exception).The tweak is to skip a
HandshakeOperationif_handshakee->is_vthread_transition_disabler() == true, so the same temporary suspension disabling mechanism would be used as for_handshakee->is_disable_suspend() == true.All other changes are to move the
is_vthread_transition_disabler()out of DEBUG to product.Testing:
Progress
Issue
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/28740/head:pull/28740$ git checkout pull/28740Update a local copy of the PR:
$ git checkout pull/28740$ git pull https://git.openjdk.org/jdk.git pull/28740/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 28740View PR using the GUI difftool:
$ git pr show -t 28740Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/28740.diff
Using Webrev
Link to Webrev Comment