On Mon, 4 Mar 2024 22:04:39 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
>> Please, review this fix correcting the JVMTI `RawMonitorWait()`
>> implementation.
>> The `RawMonitorWait()` is using the the `jt->is_interrupted(true)` to
>> update the interrupt status of the interrupted waiting thread. The issue is
>> that when it calls `jt->is_interrupted(true)` to fetch and clear the
>> `interrupt status` of the virtual thread, this information is not propagated
>> to the `java.lang.Thread` instance.
>> In the `VirtualThread` class implementation the `interrupt status` for
>> virtual thread is stored for both virtual and carrier threads. It is because
>> the implementation of object monitors for virtual threads is based on
>> pinning virtual threads, and so, always operates on carrier thread. The fix
>> is to clear the interrupt status for both virtual and carrier
>> `java.lang.Thread` instances.
>>
>> Testing:
>> - tested with new test
>> `hotspot/jtreg/serviceability/jvmti/vthread/InterruptRawMonitor` which is
>> passed with the fix and failed without it
>> - ran mach5 tiers 1-6
>
> test/hotspot/jtreg/serviceability/jvmti/vthread/InterruptRawMonitor/libInterruptRawMonitor.cpp
> line 32:
>
>> 30: static jvmtiEnv *jvmti = nullptr;
>> 31:
>> 32: static void check_thread_state(JNIEnv *jni, int check_idx) {
>
> The name of this function could be better since it only checks that the state
> is not interrupted. Maybe check_thread_not_interrupt().
Thank you. Updated as suggested.
> test/hotspot/jtreg/serviceability/jvmti/vthread/InterruptRawMonitor/libInterruptRawMonitor.cpp
> line 36:
>
>> 34:
>> 35: LOG("check #%d: Thread State: (0x%x) %s\n", check_idx, state,
>> TranslateState(state));
>> 36: if (state & JVMTI_THREAD_STATE_INTERRUPTED) {
>
> Should explicitly check `!= 0`
Thanks, fixed now.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18093#discussion_r1512178943
PR Review Comment: https://git.openjdk.org/jdk/pull/18093#discussion_r1512179056
