On Thu, 9 Jan 2025 05:03:33 GMT, Serguei Spitsyn <sspit...@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.
Changes requested by lmesnik (Reviewer).
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.
-------------
PR Review: https://git.openjdk.org/jdk/pull/22997#pullrequestreview-2540922332
PR Review Comment: https://git.openjdk.org/jdk/pull/22997#discussion_r1909428470
