On Tue, 28 Mar 2023 18:57:23 GMT, Serguei Spitsyn <sspit...@openjdk.org> wrote:

>> The fix is to enable virtual threads support for late binding JVMTI agents.
>> The fix includes:
>> - New function `JvmtiEnvBase::enable_virtual_threads_notify_jvmti()` which 
>> does enabling JVMTI VTMS transition notifications in case of agent loaded 
>> into running VM. This function executes a VM operation counting VTMS 
>> transition bits in all `JavaThread`'s to correctly set the static counter 
>> `_VTMS_transition_count` needed for VTMS transition protocol.
>> - New function `JvmtiEnvBase::disable_virtual_threads_notify_jvmti()` which 
>> is needed for testing. It is used by the `WhiteBox` API.
>> - New WhiteBox function `WB_SetVirtualThreadsNotifyJvmtiMode(JNIEnv* env, 
>> jobject wb, jboolean enable)` needed for testing of this update.
>> - New regression test: `serviceability/jvmti/vthread/ToggleNotifyJvmtiTest`
>> 
>> Testing:
>> - New test: `serviceability/jvmti/vthread/ToggleNotifyJvmtiTest`
>> - The originally failed tests are expected to pass now:
>>   `runtime/vthread/RedefineClass.java`
>>   `runtime/vthread/TestObjectAllocationSampleEvent.java` 
>> - In progress: Run the tiers 1-6 to make sure there are no regression.
>
> Serguei Spitsyn has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   fixed trailing spaces in two files

src/hotspot/share/prims/jvmtiEnvBase.cpp line 1550:

> 1548: 
> 1549:       if (jt->is_in_VTMS_transition()) {
> 1550:         count++;

For those threads that are in a transition when we enable the events, shouldn't 
we also set the jvmti_is_in_VTMS_transition field for the corresponding vthread 
as we do in JvmtiVTMSTransitionDisabler::start_VTMS_transition()? It seems a 
JvmtiVTMSTransitionDisabler that targets that particular vthread could 
otherwise proceed after the safepoint while that vthread is still in the 
transition. The "all" JvmtiVTMSTransitionDisabler won't proceed because that 
one does check the _VTMS_transition_count counter. I see that in general we 
won't have access to the vthread oop though.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/13133#discussion_r1151313147

Reply via email to