On Mon, 29 Jul 2024 22:34:46 GMT, Serguei Spitsyn <sspit...@openjdk.org> wrote:
>> Jiawei Tang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> changes according to reviewers' advice
>
> src/hotspot/share/prims/jvmtiExport.cpp line 970:
>
>> 968: if (_thread->is_in_any_VTMS_transition()) {
>> 969: return; // no events should be posted if thread is in any VTMS
>> transition
>> 970: }
>
> This is not right place to fix it.
>
> This would be better:
>
> @@ -1091,8 +1091,8 @@ bool JvmtiExport::post_class_file_load_hook(Symbol*
> h_name,
> if (JvmtiEnv::get_phase() < JVMTI_PHASE_PRIMORDIAL) {
> return false;
> }
> - if (JavaThread::current()->is_in_tmp_VTMS_transition()) {
> - return false; // skip CFLH events in tmp VTMS transition
> + if (thread->is_in_any_VTMS_transition()) {
> + return; // no events should be posted if thread is in any VTMS transition
> }
>
> JvmtiClassFileLoadHookPoster poster(h_name, class_loader,
>
>
> Also, there is a check in the constructor `JvmtiClassFileLoadHookPoster()`:
>
> if (_thread->is_in_any_VTMS_transition()) {
> return; // no events should be posted if thread is in any VTMS
> transition
> }
>
> It is better to replace it with assert. With the right check in the
> `JvmtiExport::post_class_file_load_hook()` we should never call this
> constructor and `poster.post()` when in a VTMS transition.
Change it.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20373#discussion_r1696407776
