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

Fix GetThreadState to return correct state for carrier thread #16805

Conversation

dipak-bagadiya
Copy link
Contributor

@dipak-bagadiya dipak-bagadiya commented Mar 1, 2023

JVMTI GetThreadState returns 0 (invalid state) for carrier threads
when a virtual thread is mounted over them. 0 indicates that the
thread has not started but this state is invalid for the carrier
threads when they have a virtual thread mounted.

Helper function getThreadState should not be given virtual thread
objects. It should only receive platform thread objects. Instead of
targetThread->threadObject which can be a virtual thread object, it is
given targetThread->carrierThreadObject to correctly evaluate the
state of carrier threads. This change should also be compatible with
all platform threads since carrierThreadObject is set only once during
thread creation, and it stays the same until thread termination.

Related: #16690

Signed-off-by: Dipak Bagadiya dipak.bagadiya@ibm.com

@dipak-bagadiya dipak-bagadiya marked this pull request as draft March 1, 2023 16:37
@dipak-bagadiya dipak-bagadiya force-pushed the virtual-thread-suspend-resume-fix branch from fb47bda to 0078a57 Compare March 1, 2023 17:26
@dipak-bagadiya
Copy link
Contributor Author

@babsingh requesting you to please review

@babsingh babsingh changed the title Fix GetThreadState to return correct state for carrier thread. Fix GetThreadState to return correct state for carrier thread Mar 1, 2023
@babsingh babsingh marked this pull request as ready for review March 1, 2023 18:23
JVMTI GetThreadState returns 0 (invalid state) for carrier threads
when a virtual thread is mounted over them. 0 indicates that the
thread has not started but this state is invalid for the carrier
threads when they have a virtual thread mounted.

Helper function getThreadState should not be given virtual thread
objects. It should only receive platform thread objects. Instead of
targetThread->threadObject which can be a virtual thread object, it is
given targetThread->carrierThreadObject to correctly evaluate the
state of carrier threads. This change should also be compatible with
all platform threads since carrierThreadObject is set only once during
thread creation, and it stays the same until thread termination.

Related: eclipse-openj9#16690

Signed-off-by: Dipak Bagadiya <dipak.bagadiya@ibm.com>
@dipak-bagadiya dipak-bagadiya force-pushed the virtual-thread-suspend-resume-fix branch from 0078a57 to 8134e18 Compare March 2, 2023 13:37
@babsingh
Copy link
Contributor

babsingh commented Mar 2, 2023

jenkins test sanity alinux64 jdk19

@babsingh
Copy link
Contributor

babsingh commented Mar 2, 2023

jenkins test sanity xlinux jdk8

Copy link
Contributor

@babsingh babsingh left a comment

Choose a reason for hiding this comment

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

@dipak-bagadiya has verified locally that #16690 is fixed with this PR. #16690 will be closed after the test is re-enabled.

@babsingh babsingh merged commit 15a805c into eclipse-openj9:master Mar 2, 2023
dipak-bagadiya added a commit to dipak-bagadiya/aqa-tests that referenced this pull request Mar 3, 2023
Test had been fixed by eclipse-openj9/openj9#16805

Signed-off-by: Dipak Bagadiya dipak.bagadiya@ibm.com
dipak-bagadiya added a commit to dipak-bagadiya/aqa-tests that referenced this pull request Mar 3, 2023
The test has been fixed by eclipse-openj9/openj9#16805.

Signed-off-by: Dipak Bagadiya dipak.bagadiya@ibm.com
llxia pushed a commit to adoptium/aqa-tests that referenced this pull request Mar 3, 2023
The test has been fixed by eclipse-openj9/openj9#16805.

Signed-off-by: Dipak Bagadiya dipak.bagadiya@ibm.com

Signed-off-by: Dipak Bagadiya dipak.bagadiya@ibm.com
@babsingh
Copy link
Contributor

babsingh commented Mar 6, 2023

@dipak-bagadiya This fix is needed for the JDK19 release. Can you create a PR to deliver this fix in the 0.37 release branch: https://github.com/eclipse-openj9/openj9/tree/v0.37.0-release?

fyi @tajila @pshipton @JasonFengJ9

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.

2 participants