Update jvmtiHelper.c::getVMThread to support Virtual Threads

[babsingh]

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

[babsingh]

@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: https://github.com/eclipse-openj9/openj9/blob/fc5ae4e6892b7ad809270ecb981606f4095f62da/runtime/jvmti/jvmtiHelpers.c#L128-L130 https://github.com/eclipse-openj9/openj9/blob/6a2aac87ec442d1d96863265761996c9764b19f3/runtime/vm/jvmfree.c#L200-L205

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

[gacholio]

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]

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]

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]

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

[babsingh]

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]

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]

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

[tajila]

@gacholio any concerns with this

[tajila]

jenkins test sanity amac jdk19

[tajila]

jenkins compile win jdk19

[tajila]

jenkins test sanity amac jdk19

[fengxue-IS]

I've added review comments https://github.com/eclipse-openj9/openj9/pull/15766#discussion_r953142285, I think we need to address them before merging.

[gacholio]

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

[gacholio]

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

[hangshao0]

FYI @gacholio, Eric has taken over this PR and review is being done under https://github.com/eclipse-openj9/openj9/pull/15766

[gacholio]

Thanks, I'll look over there.