Support virtual threads in JVMTI Resume* and Suspend* functions

[babsingh]

This PR impacts the following JVMTI functions:

• [Resume|Suspend]Thread
• [Resume|Suspend]ThreadList
• [Resume|Suspend]AllVirtualThreads
• GetThreadState

The above functions have been either implemented or updated as per the JVMTI doc: https://download.java.net/java/early_access/jdk19/docs/specs/jvmti.html

Other changes:

• JVM_VirtualThread* functions acquire liveVirtualThreadListMutex first, and then VM access. This allows [Resume|Suspend]AllVirtualThreads to hold liveVirtualThreadListMutex without any hangs, where VM access is not released by the JVM_VirtualThread* functions and the JVMTI functions are trying to acquire VM access to suspend the threads.
• For virtual threads, targetThread is non-null for PARKING and YIELDING states. getVirtualThreadState has been updated to use carrier thread's state whenever targetThread is non-null for a virtual thread. This correctly returns the JVMTI_THREAD_STATE_SUSPENDED flag for virtual threads.

Related: #15760

Signed-off-by: Babneet Singh [email protected]

[babsingh]

Encountering hangs; debugging on my end. @fengxue-IS Let me know if you see anything out-of-order in these changes.

[fengxue-IS]

Are you seeing the hang on just suspend/resume or suspendAll/ResumeAll ?

[babsingh]

Are you seeing the hang on just suspend/resume or suspendAll/ResumeAll ?

suspendAll/ResumeAll at the below location: https://github.com/eclipse-openj9/openj9/blob/ef6948124a16e50c03faa7e69383cb9677bc985d/runtime/jvmti/suspendhelper.cpp#L53

[babsingh]

Few issues:

1. Can't hold liveVirtualThreadListMutex while suspending/resuming virtual threads. May need two locks: one for the inspection counter and other for handling the virtual thread list. JVMTI getVMThread releases the liveVirtualThreadListMutex and waits while the vthread goes out of the unstable code regions. During this phase, new virtual threads can be added while suspending all vthreads and some may get missed from getting suspended.
2. Including the RUNNING state, targetThread is also not NULL if a vthread is in the PARKING or YIELDING states. In such states, if a vthread is blocked on liveVirtualThreadListMutex at UnMountBegin and a JVMTI thread, which has acquired the inspection rights (incremented the counter), tries to suspend the targetThread, then the targetThread will not be able to reach a safe-point for suspension via the publicFlags, and there will be a hang.
3. If we are to suspend vthreads, they can't be blocked at UnMountBegin and MountBegin. After the JVMTI releases the vthread after inspection, the threads may either start running again (MountBegin) or may hang (UnMountBegin).

@tajila @fengxue-IS @gacholio If you have quick & easy solutions, please share them here. Also, let me know if I have misinterpreted anything.

[babsingh]

The following tests have successfully passed:

• https://github.com/ibmruntimes/openj9-openjdk-jdk19/tree/openj9/test/hotspot/jtreg/serviceability/jvmti/vthread/SuspendResume2
• https://github.com/ibmruntimes/openj9-openjdk-jdk19/tree/openj9/test/hotspot/jtreg/serviceability/jvmti/thread/GetThreadState

Note: Testing had to be run with -Xint -Xgcpolicy:nogc since segfaults originating from the GC and hangs from the JIT were seen.

The following tests could not be run successfully since some of the functions haven't been implemented:

• https://github.com/ibmruntimes/openj9-openjdk-jdk19/tree/openj9/test/hotspot/jtreg/serviceability/jvmti/vthread/SuspendResume1 [GetThreadListStackTraces has not been updated for vthreads yet]
• https://github.com/ibmruntimes/openj9-openjdk-jdk19/tree/openj9/test/hotspot/jtreg/serviceability/jvmti/vthread/SuspendResumeAll [Issue with deriving carrier threads]

[babsingh]

@keithc-ca All of your feedback has been addressed. Also, removed the draft state. Can you please complete your review?

[babsingh]

@keithc-ca I have addressed all the comments in the last feedback. But I will have to rework my changes because a deadlock was found while running the non-public cert tests last night. The rework will include adding a new lock. One lock will explicitly manage the vThreadInspectCount, and the other lock will explicitly handle the vThreadList. None of the locks will be held if there is a halt on internalEnterVMFromJNI/internalExitVMToJNI during suspension. The purpose of using two locks is to avoid intermittent deadlocks and improve perf by reducing the critical section's size.

[keithc-ca]

I will have to rework my changes because a deadlock was found

I'll move this to the draft state until you've addressed that.

[babsingh]

@gacholio All previous feedback has been addressed.

• Re-implemented with one monitor.
• VirtualThread.state can't be modified easily from JVMTI. For using this approach, it will require 1) invocation of Java methods from JVMTI methods in order to resume a virtual thread, and 2) modification of VirtualThread's Java code to avoid race-conditions.
• Had to add a hidden field in VirtualThread as a work-around for the VirtualThread.state approach. This avoids all the above issues. Will need your guidance on the hidden field: wanted the field to be boolean; but didn't know which macros to use for load and store.

[gacholio]

Can you please detail the issue with the state field? Boolean fields (hidden or not) are 32 bits (no field is currently ever smaller than 32 bits).

[gacholio]

I'm not sure your approach is correct - for normal threads, there is no distinction made between JVMTI or java suspending/resuming a thread. This makes the APIs completely undependable, but I suspect the new APIs are equally bad. You'll need to test/prove that JVMTI and java suspend for virtual threads either has the same issue or does not.

[babsingh]

test/prove that JVMTI and java suspend for virtual threads either has the same issue or does not.

Java resume and suspend throw UnsupportedOperationException if invoked on a virtual thread: https://download.java.net/java/early_access/loom/docs/api/java.base/java/lang/Thread.html#suspend(). The two approaches are incomparable.

Can you please detail the issue with the state field? Boolean fields (hidden or not) are 32 bits (no field is currently ever smaller than 32 bits).

Just wanted to confirm the syntax and macros to use for a boolean field. Since boolean fields are 32 bits, the below usage is the correct approach?

vmFuncs->addHiddenInstanceField(vm, "java/lang/VirtualThread", "isSuspendedByJVMTI", "Z", &vm->isSuspendedByJVMTIOffset)

J9OBJECT_U32_STORE(currentThread, threadObject, vm->isSuspendedByJVMTIOffset, TRUE);

if (J9OBJECT_U32_LOAD(currentThread, threadObject, vm->isSuspendedByJVMTIOffset)) {

[gacholio]

The macros are correct. You have not described the issue with the vthread state.

So there is no java way to suspend/resume a vthread?

[babsingh]

So there is no java way to suspend/resume a vthread?

• Internal (private) API in Java code can suspend a virtual thread during VirtualThread.tryGetStackTrace by setting the SUSPENDED flag in VirtualThread.state.
• Setting the SUSPENDED flag prevents VirtualThread.runContinuation to run the continuation. The flag is only allowed to be set if the thread is unmounted i.e. VirtualThread.state is either RUNNABLE or PARKED. A retry approach is used if the compare-and-set fails to set the SUSPENDED flag.

the issue with the vthread state.

• After removing the SUSPENDED flag, VirtualThread.submitRunContinuation needs to be invoked for rescheduling the thread. Otherwise, the thread won't run. Invoking VirtualThread.submitRunContinuation or similar Java code from JVMTI to resume a thread will be inefficient.
• If the thread is waiting in JVM_VirtualThreadMountBegin while being suspended by JVMTI, then more logic will be required because VirtualThread.submitRunContinuation or similar code should not be invoked to resume a thread in this case.
• Race condition: If JVMTI sets the SUSPENDED flag in VirtualThread.state, then VirtualThread.tryGetStackTrace may unset it before JVMTI resumes the thread.
• The hidden field approach does not depend on VirtualThread.state, and it suspends at JVM_VirtualThreadMountEnd (inside VirtualThread.run) if the thread is being mounted.

[gacholio]

Regarding the race condition above, won't that already occur if multiple threads are trying to get the stack trace, or is there synchronization to prevent this?

[babsingh]

Regarding the race condition above, won't that already occur if multiple threads are trying to get the stack trace, or is there synchronization to prevent this?

Jack says that the compare-and-set in VirtualThread.tryGetStackTrace will guarantee only one thread is allowed. Other threads will fail the compare-and-set and will have to retry.

[gacholio]

I just need to go over in detail the interaction between the suspend flag in the vthread and the halt flag to see if there are any timing holes.

[gacholio]

Are only unmounted vthreads marked as suspended using the new field?

[gacholio]

What is the justification for clearing the vthread flag when setting the publicFlag? Wouldn't things be simpler if the vhtread flag was always correct?

[babsingh]

Are only unmounted vthreads marked as suspended using the new field?

Yes. Only unmounted vthreads are marked as suspended using the new field. For mounted vthreads, the state is derived from the carrier thread's J9VMThread, and the new field is unused.

What is the justification for clearing the vthread flag when setting the publicFlag? Wouldn't things be simpler if the vhtread flag was always correct?

At this point, the vthread is successfully mounted. The vthread flag is cleared when setting the publicFlag to be consistent with how mounted and unmounted vthreads are currently marked as suspended. Setting the vthread flag for both mounted and unmounted vthreads will require an extra isVirtualThread check in jvmtiThread.c::resumeThread and suspendhelper.cpp::suspendThread.

[gacholio]

@fengxue-IS Any comments?

[gacholio]

jenkins test sanity win jdk8

[gacholio]

jenkins test sanity,extended zlinux jdk19

[fengxue-IS]

@fengxue-IS Any comments?

nothing from me, I think everything have been address