On Tue, 23 Apr 2024 19:05:38 GMT, Patricio Chilano Mateo
<pchilanom...@openjdk.org> wrote:
>> This is a simple fix of three similar asserts.
>> The `_target_jt->jvmti_vthread()` has to be used instead of
>> `_target_jt->vthread()`.
>> The `_target_jt->vthread()` can be outdated in some specific contexts as
>> shown in the `hs_err` stack trace.
>>
>> I've seen similar issue and already fixed it in this fragment of code:
>>
>> class GetCurrentLocationClosure : public JvmtiUnitedHandshakeClosure {
>> . . .
>> void do_vthread(Handle target_h) {
>> assert(_target_jt == nullptr || !_target_jt->is_exiting(), "sanity
>> check");
>> // use jvmti_vthread() as vthread() can be outdated
>> assert(_target_jt == nullptr || _target_jt->jvmti_vthread() ==
>> target_h(), "sanity check");
>> . . .
>>
>> The issue above was fixed by replacing `_target_jt->vthread()` with
>> `_target_jt->jvmti_vthread()`.
>>
>> There are three places which need to be fixed the same way:
>> - `GetSingleStackTraceClosure::do_vthread(Handle target_h)`
>> - `SetForceEarlyReturn::do_vthread(Handle target_h)`
>> - `UpdateForPopTopFrameClosure::do_vthread(Handle target_h)`
>>
>> Testing:
>> - Run mach5 tiers 1-6
>
> src/hotspot/share/prims/jvmtiEnvBase.cpp line 2079:
>
>> 2077: void
>> 2078: GetSingleStackTraceClosure::do_vthread(Handle target_h) {
>> 2079: // use jvmti_vthread() as vthread() can be outdated
>
> The only reason I can see of why just using vthread() doesn't work is because
> of the case where we are in a temporary switch to carrier thread. So maybe
> change comment to be: "use jvmti_vthread() instead of vthread() as target
> could have temporary changed identity to carrier thread (see
> VirtualThread.switchToCarrierThread)". Same in the other places.
Thank you for the suggestion. Will fix.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18806#discussion_r1576959637
