Block Virtual Threads for JVMTI Inspection #15690

babsingh · 2022-08-09T20:45:07Z

Return the corresponding J9VMThread for a virtual thread from
getVMThread.
Prevent the virtual thread from unmounting using the inspectorCount
approach.
The virtual thread is stopped just before J9VMThread->threadObject is set
from the virtual thread to the carrier thread.
This also prevents a virtual thread from freeing its native resources while
it is being inspected.
For a virtual thread, even the underlying J9VMThread is prevented from
freeing its native resources using the inspectorCount approach.
isSameOrSuperClassOf is unable to handle NULL from the *_OR_NULL
version. Instead, use J9VMJAVALANGBASEVIRTUALTHREAD in
IS_VIRTUAL_THREAD.
Moved code from notifyJvmtiUnmountEnd to notifyJvmtiUnmountBegin for blocking
before unmount. Also, VirtualThread.carrierThread is set to NULL in unmount.
So, removing the VirtualThread from the linked list before the last unmount
in notifyJvmtiUnmountBegin seems correct.
Block mount after yield if a virtual thread is being inspected.
In getVMThread:
- targetThread will be NULL for a yielded virtual thread.
- JVMTI_ERROR_NONE will be returned for a yielded virtual thread.
- virtualThreadInspectorCount will be incremented for a yielded
  virtual thread.
Reload references after the wait since they can become outdated if a GC
occurs during the wait.
Release VM access before the wait and re-acquire VM access after the wait.
Decrement virtualThreadInspectorCount for yielded virtual threads i.e.
(targetThread == NULL).

Related: #15183. This impacts all JVMTI functions that rely on getVMThread and
releaseVMThread.

Signed-off-by: Babneet Singh sbabneet@ca.ibm.com

babsingh · 2022-08-10T15:41:28Z

@gacholio When a virtual thread is being inspected in JVMTI, do we need to protect the virtual thread and corresponding carrier thread from freeing their native resources such as the new TLS?

Existing code to prevent freeing of J9VMThread native resources:

openj9/runtime/jvmti/jvmtiHelpers.c

Lines 128 to 130 in fc5ae4e

 *vmThreadPtr = targetThread; 

 if (targetThread != NULL) { 

 ++(targetThread->inspectorCount);

openj9/runtime/vm/jvmfree.c

Lines 200 to 205 in 6a2aac8

 /* If this thread is being inspected, do not allow it to die */ 

 omrthread_monitor_enter(vm->vmThreadListMutex); 

 while (vmThread->inspectorCount != 0) { 

 omrthread_monitor_wait(vm->vmThreadListMutex); 

 }

Similar code will be needed to prevent freeing of virtual thread resources?

runtime/jvmti/jvmtiHelpers.c

gacholio · 2022-08-11T19:14:12Z

Is the unmount native called before the vthread ends and when it yields? Both of these need to block if the vthread is being inspected.

Also wondering if haltThreadForInspection will also need to be modified in a similar fashion.

babsingh · 2022-08-11T21:20:25Z

Is the unmount native called before the vthread ends and when it yields? Both of these need to block if the vthread is being inspected.

Yes, notifyJvmtiUnmountBegin is called before the vthread ends and when it yields. For the yield case, the boolean input parameter is false. Updated notifyJvmtiUnmountBegin to block in both cases.

References:

Terminate: VirtualThread.java#L305
Yield: VirtualThread.java#L367

babsingh · 2022-08-11T21:29:10Z

Also wondering if haltThreadForInspection will also need to be modified in a similar fashion.

haltThreadForInspection is always called after getVMThread. So, haltThreadForInspection should work with virtual threads as-is because both the virtual thread and corresponding carrier thread (J9VMThread) are simultaneously blocked/pinned in getVMThread. Also, halting the carrier thread should automatically halt the mounted virtual thread. ++@tajila @fengxue-IS Comment if you disagree.

gacholio · 2022-08-11T21:29:56Z

The native should be called before the yield (so that the vmthread stack still represents the continuation, not the carrier).

babsingh · 2022-08-11T21:31:57Z

The native should be called before the yield (so that the vmthread stack still represents the continuation, not the carrier).

This is true. See VirtualThread.java#L367. The native (notifyJvmtiUnmountBegin) is called before Continuation.yield.

gacholio · 2022-08-11T21:33:22Z

haltThreadForInspection is called in many places outside of JVMTI, but we can ignore it for now. If a thread is halted, it will be unable to change the mount state.

babsingh · 2022-08-11T21:36:09Z

@EricYangIBM Can you review these changes? Also, let me know if they work correctly with your TLS changes and related tests.

runtime/jvmti/jvmtiHelpers.c

runtime/jcl/common/thread.cpp

runtime/jvmti/jvmtiHelpers.c

runtime/jcl/common/thread.cpp

runtime/jvmti/jvmtiHelpers.c

- Return the corresponding J9VMThread for a virtual thread from getVMThread. - Prevent the virtual thread from unmounting using the inspectorCount approach. - The virtual thread is stopped just before J9VMThread->threadObject is set from the virtual thread to the carrier thread. - This also prevents a virtual thread from freeing its native resources while it is being inspected. - For a virtual thread, even the underlying J9VMThread is prevented from freeing its native resources using the inspectorCount approach. - isSameOrSuperClassOf is unable to handle NULL from the *_OR_NULL version. Instead, use J9VMJAVALANGBASEVIRTUALTHREAD in IS_VIRTUAL_THREAD. - Moved code from notifyJvmtiUnmountEnd to notifyJvmtiUnmountBegin for blocking before unmount. Also, VirtualThread.carrierThread is set to NULL in unmount. So, removing the VirtualThread from the linked list before the last unmount in notifyJvmtiUnmountBegin seems correct. - Block mount after yield if a virtual thread is being inspected. - In getVMThread: - targetThread will be NULL for a yielded virtual thread. - JVMTI_ERROR_NONE will be returned for a yielded virtual thread. - virtualThreadInspectorCount will be incremented for the yielded virtual thread. - Reload references after the wait since they can become outdated if a GC occurs during the wait. - Release VM access before the wait and re-acquire VM access after the wait. - Decrement virtualThreadInspectorCount for yielded virtual threads i.e. (targetThread == NULL). Related: eclipse-openj9#15183. This impacts all JVMTI functions that rely on getVMThread and releaseVMThread. Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>

tajila · 2022-08-23T14:23:36Z

@gacholio any concerns with this

tajila · 2022-08-23T14:23:48Z

jenkins test sanity amac jdk19

tajila · 2022-08-23T14:24:00Z

jenkins compile win jdk19

tajila · 2022-08-23T14:36:31Z

jenkins test sanity amac jdk19

fengxue-IS · 2022-08-23T21:59:01Z

I've added review comments #15766 (comment), I think we need to address them before merging.

gacholio · 2022-08-24T16:35:24Z

runtime/jcl/common/thread.cpp


-  omrthread_monitor_enter(vm->liveVirtualThreadListMutex);
+ if (firstMount) {


Why is the count not checked in this case?

gacholio · 2022-08-24T16:42:27Z

I see a lot of releaseVMThread being called with NULL thread - why is this? Won't it leave the counter unbalanced?

gacholio · 2022-08-24T16:43:07Z

This same code needs to go in haltThreadForInspection, I think, so the comments in the general code should not reference JVMTI.

hangshao0 · 2022-08-24T18:03:13Z

FYI @gacholio, Eric has taken over this PR and review is being done under #15766

gacholio · 2022-08-24T18:56:01Z

Thanks, I'll look over there.

- Return the corresponding J9VMThread for a virtual thread from getVMThread. - Prevent the virtual thread from (un)mounting using the inspectorCount approach. - The (un)mountBegin/(un)mountEnd natives lock the mount/unmount section and prevent inspectors from inspecting during that time. - For a virtual thread, even the underlying J9VMThread is prevented from terminating using the inspectorCount approach. - Moved code from notifyJvmtiUnmountEnd to notifyJvmtiUnmountBegin for blocking before unmount. Also, VirtualThread.carrierThread is set to NULL in unmount. So, removing the VirtualThread from the linked list before the last unmount in notifyJvmtiUnmountBegin seems correct. - Block mount after yield if a virtual thread is being inspected. - In getVMThread: - targetThread will be NULL for a yielded virtual thread. - JVMTI_ERROR_NONE will be returned for a yielded virtual thread. - virtualThreadInspectorCount will be incremented for the yielded virtual thread. - If a virtual thread is in the process of mounting/unmounting, wait until it is done before inspecting. Related: eclipse-openj9#15183. This impacts all JVMTI functions that rely on getVMThread and releaseVMThread. Original PR: eclipse-openj9#15690 Co-authored-by: Babneet Singh <sbabneet@ca.ibm.com> Signed-off-by: Eric Yang <eric.yang@ibm.com>

babsingh force-pushed the loom_jvmti_1 branch 3 times, most recently from 9123109 to 1a8072f Compare August 9, 2022 20:53

babsingh force-pushed the loom_jvmti_1 branch 6 times, most recently from 25458f3 to 61d9715 Compare August 11, 2022 13:42

This was referenced Aug 11, 2022

Loom: JVMTI support #15183

Closed

Enable VirtualThread natives by default ibmruntimes/openj9-openjdk-jdk19#18

Closed

babsingh force-pushed the loom_jvmti_1 branch from 61d9715 to d426cc2 Compare August 11, 2022 16:18

gacholio reviewed Aug 11, 2022

View reviewed changes

runtime/jvmti/jvmtiHelpers.c Outdated Show resolved Hide resolved

babsingh force-pushed the loom_jvmti_1 branch from d426cc2 to 359e035 Compare August 11, 2022 18:45

babsingh force-pushed the loom_jvmti_1 branch from 359e035 to 530675d Compare August 11, 2022 21:16

babsingh marked this pull request as ready for review August 11, 2022 21:40

EricYangIBM reviewed Aug 12, 2022

View reviewed changes

runtime/jvmti/jvmtiHelpers.c Outdated Show resolved Hide resolved

babsingh force-pushed the loom_jvmti_1 branch 3 times, most recently from 1a10f91 to 9d2ec78 Compare August 16, 2022 15:47

tajila reviewed Aug 18, 2022

View reviewed changes

runtime/jcl/common/thread.cpp Show resolved Hide resolved

babsingh force-pushed the loom_jvmti_1 branch from 9d2ec78 to 6023776 Compare August 19, 2022 05:00

hangshao0 reviewed Aug 19, 2022

View reviewed changes

runtime/jvmti/jvmtiHelpers.c Outdated Show resolved Hide resolved

runtime/jcl/common/thread.cpp Outdated Show resolved Hide resolved

runtime/jvmti/jvmtiHelpers.c Outdated Show resolved Hide resolved

babsingh force-pushed the loom_jvmti_1 branch from 7276212 to 354eb8f Compare August 19, 2022 19:54

hangshao0 reviewed Aug 19, 2022

View reviewed changes

runtime/jvmti/jvmtiHelpers.c Outdated Show resolved Hide resolved

babsingh force-pushed the loom_jvmti_1 branch 3 times, most recently from 619d3b6 to 260af17 Compare August 19, 2022 21:56

babsingh force-pushed the loom_jvmti_1 branch 3 times, most recently from 32df9a3 to e55f36e Compare August 19, 2022 23:38

babsingh mentioned this pull request Aug 19, 2022

Use JVM_VirtualThread* functions correctly with Virtual Threads #15755

Merged

EricYangIBM mentioned this pull request Aug 22, 2022

Block Virtual Threads for JVMTI Inspection #15766

Merged

hangshao0 approved these changes Aug 22, 2022

View reviewed changes

tajila approved these changes Aug 23, 2022

View reviewed changes

gacholio reviewed Aug 24, 2022

View reviewed changes

gacholio closed this Aug 24, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Block Virtual Threads for JVMTI Inspection #15690

Block Virtual Threads for JVMTI Inspection #15690

babsingh commented Aug 9, 2022 •

edited

Loading

babsingh commented Aug 10, 2022 •

edited

Loading

gacholio commented Aug 11, 2022

babsingh commented Aug 11, 2022 •

edited

Loading

babsingh commented Aug 11, 2022 •

edited

Loading

gacholio commented Aug 11, 2022

babsingh commented Aug 11, 2022

gacholio commented Aug 11, 2022

babsingh commented Aug 11, 2022

tajila commented Aug 23, 2022

tajila commented Aug 23, 2022

tajila commented Aug 23, 2022

tajila commented Aug 23, 2022

fengxue-IS commented Aug 23, 2022

gacholio Aug 24, 2022

gacholio commented Aug 24, 2022

gacholio commented Aug 24, 2022

hangshao0 commented Aug 24, 2022

gacholio commented Aug 24, 2022


		omrthread_monitor_enter(vm->liveVirtualThreadListMutex);
		if (firstMount) {

Block Virtual Threads for JVMTI Inspection #15690

Block Virtual Threads for JVMTI Inspection #15690

Conversation

babsingh commented Aug 9, 2022 • edited Loading

babsingh commented Aug 10, 2022 • edited Loading

gacholio commented Aug 11, 2022

babsingh commented Aug 11, 2022 • edited Loading

babsingh commented Aug 11, 2022 • edited Loading

gacholio commented Aug 11, 2022

babsingh commented Aug 11, 2022

gacholio commented Aug 11, 2022

babsingh commented Aug 11, 2022

tajila commented Aug 23, 2022

tajila commented Aug 23, 2022

tajila commented Aug 23, 2022

tajila commented Aug 23, 2022

fengxue-IS commented Aug 23, 2022

gacholio Aug 24, 2022

Choose a reason for hiding this comment

gacholio commented Aug 24, 2022

gacholio commented Aug 24, 2022

hangshao0 commented Aug 24, 2022

gacholio commented Aug 24, 2022

babsingh commented Aug 9, 2022 •

edited

Loading

babsingh commented Aug 10, 2022 •

edited

Loading

babsingh commented Aug 11, 2022 •

edited

Loading

babsingh commented Aug 11, 2022 •

edited

Loading