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 to handle suspend/resume of virtual/carrier threads #17350

Merged

Conversation

dipak-bagadiya
Copy link
Contributor

JVMTI treats a virtual thread and its carrier thread as two separate threads. If JVMTI suspends a virtual thread, then its carrier thread should not be suspended and vice-versa. Similarly, if JVMTI resumes a virtual thread, then its carrier thread should not be resumed and vice-versa.
In OpenJ9, currently, a mounted virtual thread and its carrier thread shared the same J9VMThread. The thread state is derived from the J9VMThread. Due to the sharing of the J9VMThread in the mounted state,if a mounted virtual thread is suspended, then the JVMTI functions will also reflect that its carrier thread is suspended.

Currently, "isSuspendedByJVMTI" is the hidden field that holds the suspend status for the virtual thread only, now it made available for carrier thread too, i.e. the scope has changed from "java/lang/VirtualThread" to "java/lang/Thread" threads Also, it is renamed to "isSuspendedInternal" as this hidden field is not just specific to JVMTI.

The following functions has changed to set/reset and verify "isSuspendedInternal" status for threads:-

  1. SUSPEND THREAD:-
    suspendhelper.cpp:suspendThread:
    jvmtiThread.c:jvmtiSuspendResumeCallBack
    thread.cpp:Java_java_lang_Thread_suspendImpl
    ->In case of suspend the halt flag
    (J9_PUBLIC_FLAGS_HALT_THREAD_JAVA_SUSPEND) is set if the thread is mounted, i.e. threadObject == targetThread->threadObject in J9VMThread's publicFlags also the hidden field "isSuspendedInternal" set to a non-zero value for the thread instance in all cases regardless of thread being mounted or unmounted.
    ->If any of the public flags are already set, then the relevant failure message is assigned.

  2. RESUME THREAD:-
    jvmtiThread.c:resumeThread:
    jvmtiThread.c:jvmtiSuspendResumeCallBack
    thread.cpp:Java_java_lang_Thread_resumeImpl
    ->In case of resume the halt flag
    (J9_PUBLIC_FLAGS_HALT_THREAD_JAVA_SUSPEND) is cleared if the thread is mounted, i.e. threadObject == targetThread->threadObject in J9VMThread's publicFlags also the hidden field "isSuspendedInternal" set to a zero value for the thread instance in all cases regardless of thread being mounted or unmounted.
    ->If any of the public flags are already set, then the relevant failure message is assigned.

  3. GETSTATE:-
    jvmtiHelpers.c:getThreadState
    jvmtiHelpers.c:getVirtualThreadState
    ->The getThreadState function is changed in such a way that will verify the "isSuspendedInternal" filed to check the thread is suspend or not.

The basic concept behind the new changes:
[mounted + unmounted] set the "isSuspendedInternal" field
[mounted] the halt flag is only modified if the thread object is
stored in targetThread->threadObject
[unmounted] Delay setting the halt flag until the thread mount.

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

JVMTI treats a virtual thread and its carrier thread as two separate
threads. If JVMTI suspends a virtual thread, then its carrier thread
should not be suspended and vice-versa. Similarly, if JVMTI resumes
a virtual thread, then its carrier thread should not be resumed and
vice-versa.
In OpenJ9, currently, a mounted virtual thread and its carrier thread
shared the same J9VMThread. The thread state is derived from the
J9VMThread. Due to the sharing of the J9VMThread in the mounted
state,if a mounted virtual thread is suspended, then the JVMTI
functions will also reflect that its carrier thread is suspended.

Currently, "isSuspendedByJVMTI" is the hidden field that holds
the suspend status for the virtual thread only, now it made
available for carrier thread too, i.e. the scope has changed
from "java/lang/VirtualThread" to "java/lang/Thread" threads
Also, it is renamed to "isSuspendedInternal" as this hidden field is
not just specific to JVMTI.

The following functions has changed to set/reset and verify
"isSuspendedInternal" status for threads:-

1) SUSPEND THREAD:-
suspendhelper.cpp:suspendThread:
jvmtiThread.c:jvmtiSuspendResumeCallBack
thread.cpp:Java_java_lang_Thread_suspendImpl
->In case of suspend the halt flag
(J9_PUBLIC_FLAGS_HALT_THREAD_JAVA_SUSPEND) is set if the thread is
mounted, i.e. threadObject == targetThread->threadObject in
J9VMThread's publicFlags also the hidden field "isSuspendedInternal"
set to a non-zero value for the thread instance in all cases
regardless of thread being mounted or unmounted.
->If any of the public flags are already set, then the relevant
failure message is assigned.

2) RESUME THREAD:-
jvmtiThread.c:resumeThread:
jvmtiThread.c:jvmtiSuspendResumeCallBack
thread.cpp:Java_java_lang_Thread_resumeImpl
->In case of resume the halt flag
(J9_PUBLIC_FLAGS_HALT_THREAD_JAVA_SUSPEND) is cleared if the thread
is mounted, i.e. threadObject == targetThread->threadObject in
J9VMThread's publicFlags also the hidden field "isSuspendedInternal"
set to a zero value for the thread instance in all cases regardless
of thread being mounted or unmounted.
->If any of the public flags are already set, then the relevant
failure message is assigned.

3) GETSTATE:-
jvmtiHelpers.c:getThreadState
jvmtiHelpers.c:getVirtualThreadState
->The getThreadState function is changed in such a way that will
verify the "isSuspendedInternal" filed to check the thread is
suspend or not.

The basic concept behind the new changes:
[mounted + unmounted] set the "isSuspendedInternal" field
[mounted]   the halt flag is only modified if the thread object is
	    stored in targetThread->threadObject
[unmounted] Delay setting the halt flag until the thread mount.

Related: eclipse-openj9#16689
Signed-off-by: Dipak Bagadiya dipak.bagadiya@ibm.com
@dipak-bagadiya dipak-bagadiya marked this pull request as ready for review May 8, 2023 12:02
@dipak-bagadiya
Copy link
Contributor Author

@babsingh

The personal build for JDK 8, 11, and 20 was triggered, and the list of known failures is listed below; no other failures were noticed.

The below failure appears to be known ones, but I did not find any open issue for the same reason, and it
appear to be unrelated to the changes in this PR.

  • java/util/stream/test/org/openjdk/tests/java/util/stream/FlatMapOpTest.java.FlatMapOpTest

@babsingh
Copy link
Contributor

jenkins test sanity,sanity.openjdk,extended.openjdk zlinux jdk20

@babsingh
Copy link
Contributor

jenkins test sanity,sanity.openjdk,extended.openjdk alinux64 jdk17

@babsingh
Copy link
Contributor

jenkins test sanity,sanity.openjdk,extended.openjdk win jdk20

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.

@babsingh babsingh merged commit 3e7e8f9 into eclipse-openj9:master May 10, 2023
dipak-bagadiya added a commit to dipak-bagadiya/aqa-tests that referenced this pull request May 11, 2023
Test has been fixed by eclipse-openj9/openj9#17350

Signed-off-by: Dipak Bagadiya dipak.bagadiya@ibm.com
llxia added a commit to adoptium/aqa-tests that referenced this pull request May 16, 2023
Test has been fixed by eclipse-openj9/openj9#17350

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

Signed-off-by: Dipak Bagadiya dipak.bagadiya@ibm.com
Co-authored-by: Lan Xia <Lan_Xia@ca.ibm.com>
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