On Thu, 9 Jan 2025 21:01:19 GMT, Leonid Mesnik <lmes...@openjdk.org> wrote:
>> This is a fix of one more deadlock issue related to `interruptLock` critical
>> sections. When the `interruptLock` is hold by the target virtual thread it
>> is unsafe to suspend or post JVMTI events. This update is to ignore the
>> JVMTI events when the `interruptLock` is hold. It is additionally to the
>> cases when the target JavaThread is in a `VTMS` transition. It is based on
>> the existing mechanism disallowing JVMTI suspends while the `interruptLock`
>> is hold. In order to support this the target JavaThread has the
>> `_is_disable_suspend` bit.
>>
>> Testing:
>> - Ran mach5 tiers 1-6
>> - There is no regression test for this issue as it is not hard to construct
>> one. Originally, the issue was reported by JGroups which is using the
>> `Async` profiler.
>
> src/hotspot/share/runtime/javaThread.hpp line 724:
>
>> 722: void set_VTMS_transition_mark(bool val) {
>> Atomic::store(&_VTMS_transition_mark, val); }
>> 723:
>> 724: bool hide_jvmti_events() const { return
>> _is_in_VTMS_transition || _is_disable_suspend; }
>
> The name
> hide_jvmti_events()
> looks incorrect, like we change thread state to hide jvmti event and
> fail/return if can't.
> I prefer something like
> jvmti_events_hidden()
> is_jvmti_event_posting_hidden()
> or
> should_hide_jvmti_events()
> To make clear that we read only some state.
> Also, agree with Chris that comments are needed only in the implementation of
> this method.
>
> Good to know when thread can and can't post events.
Thank you for the comment.
I like this suggestion: `should_hide_jvmti_events()`.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22997#discussion_r1909668564
