Skip to content
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

Spin during VirtualThread MountBegin and UnmountBegin #18439

Merged
merged 1 commit into from
Nov 13, 2023

Conversation

babsingh
Copy link
Contributor

Currently, the halt flag is set in VirtualThread MountEnd if a virtual
thread is suspended via JVMTI, and in VirtualThread UnmountEnd if a
carrier thread is suspended via JVMTI.

In the above approach, the halt flag is set too late. As soon as the
continuation swaps the J9VMThread context, the thread begins execution
and is capable of triggering JVMTI events.

To avoid the above issue, the above steps are moved into VirtualThread
MountBegin and UnmountBegin. This prevents the continuation to swap
the J9VMThread context.

Currently, the halt flag is set without invoking
exitVThreadTransitionCritical. This prevents JVMTI to resume the halted
thread and cause a hang. The new approach spins, invokes
exitVThreadTransitionCritical and releases VM access to allow JVMTI to
resume the suspended thread.

The better approach will be to fail mount if the thread is suspended
and retry later. Currently, his approach cannot be implemented because
VirtualThread.java does not support this approach.

Related: #17865
Related: #17869
Related: #18370

@babsingh
Copy link
Contributor Author

@fengxue-IS Requesting your review.

fyi @tajila

Copy link
Contributor

@fengxue-IS fengxue-IS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code LGTM, only question is about moving carrier suspend to unmountBegin means that the threadObject would still point to vthread. Would this affect any other JVMTI API?

In the above approach, the halt flag is set too late. As soon as the
continuation swaps the J9VMThread context, the thread begins execution
and is capable of triggering JVMTI events.

My understanding is that J9_PRIVATE_FLAGS_VIRTUAL_THREAD_HIDDEN_FRAMES flag set during mount/unmountBegin should prevent JVMTI events from being dispatched. are there cases which have not been covered?

@babsingh
Copy link
Contributor Author

code LGTM, only question is about moving carrier suspend to unmountBegin means that the threadObject would still point to vthread. Would this affect any other JVMTI API?

i don't believe so. carrier thread will stay suspended. the virtual will look as if it is still running. if someone suspends the virtual thread, it will halt while re-acquiring VM access.

My understanding is that J9_PRIVATE_FLAGS_VIRTUAL_THREAD_HIDDEN_FRAMES flag set during mount/unmountBegin should prevent JVMTI events from being dispatched. are there cases which have not been covered?

in #16654, we didn't disable all JVMTI events with J9_PRIVATE_FLAGS_VIRTUAL_THREAD_HIDDEN_FRAMES. when a thread is suspended, this change will prevent all JVMTI events.

@babsingh
Copy link
Contributor Author

jenkins test sanity.functional,sanity.openjdk,extended.openjdk aix,zlinux jdk21

Currently, the halt flag is set in VirtualThread MountEnd if a virtual
thread is suspended via JVMTI, and in VirtualThread UnmountEnd if a
carrier thread is suspended via JVMTI.

In the above approach, the halt flag is set too late. As soon as the
continuation swaps the J9VMThread context, the thread begins execution
and is capable of triggering JVMTI events.

To avoid the above issue, the above steps are moved into VirtualThread
MountBegin and UnmountBegin. This prevents the continuation to swap
the J9VMThread context.

Currently, the halt flag is set without invoking
exitVThreadTransitionCritical. This prevents JVMTI to resume the halted
thread and cause a hang. The new approach spins, invokes
exitVThreadTransitionCritical  and releases VM access to allow JVMTI to
resume the suspended thread.

The better approach will be to fail mount if the thread is suspended
and retry later. Currently, his approach cannot be implemented because
VirtualThread.java does not support this approach.

Related: eclipse-openj9#17865
Related: eclipse-openj9#17869
Related: eclipse-openj9#18370

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
@babsingh
Copy link
Contributor Author

Old PR build: https://openj9-jenkins.osuosl.org/job/PullRequest-OpenJ9/4736. It should still be good since the last update only added the TODO comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants