RFR: 8329674: JvmtiEnvThreadState::reset_current_location function should use JvmtiHandshake [v2]

[Patricio Chilano Mateo]

On Wed, 10 Apr 2024 04:21:23 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

>> The internal JVM TI JvmtiHandshake and JvmtiUnitedHandshakeClosure classes were introduced in the JDK 22 to unify/simplify the JVM TI functions supporting implementation of the virtual threads. This enhancement is to refactor the JVM TI internal functions JvmtiEnvThreadState::reset_current_location on the base of JvmtiHandshake and JvmtiUnitedHandshakeClosure classes.
>> 
>> Testing:
>>  - Ran mach5 tiers 1-6
>
> Serguei Spitsyn has updated the pull request incrementally with one additional commit since the last revision:
> 
>   review: refactored to get rid of overloaded doit functions

Looks good to me, just a few comments.

src/hotspot/share/prims/jvmtiEnvThreadState.cpp line 307:

> 305:     if (!JvmtiEnvBase::is_vthread_alive(target_h())) {
> 306:       return; // _completed remains false.
> 307:     }

Do we need this? We already do this check in JvmtiHandshake::execute().

src/hotspot/share/prims/jvmtiEnvThreadState.cpp line 309:

> 307:     }
> 308:     ResourceMark rm;
> 309:     javaVFrame *jvf = JvmtiEnvBase::get_vthread_jvf(target_h());

This method already handles both mounted and unmounted case, so do we need the first conditional above?

src/hotspot/share/prims/jvmtiEnvThreadState.cpp line 367:

> 365:       GetCurrentLocationClosure op;
> 366:       JvmtiHandshake::execute(&op, &tlh, thread, thread_h);
> 367: 

Seems we are missing a JvmtiVTMSTransitionDisabler.

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

PR Review: https://git.openjdk.org/jdk/pull/18630#pullrequestreview-1994658919
PR Review Comment: https://git.openjdk.org/jdk/pull/18630#discussion_r1561295746
PR Review Comment: https://git.openjdk.org/jdk/pull/18630#discussion_r1561298952
PR Review Comment: https://git.openjdk.org/jdk/pull/18630#discussion_r1561300541