On Fri, 10 Mar 2023 17:06:58 GMT, Patricio Chilano Mateo
<[email protected]> wrote:
>> src/hotspot/share/prims/jvmtiThreadState.cpp line 372:
>>
>>> 370: java_lang_Thread::dec_VTMS_transition_disable_count(vth());
>>> 371: Atomic::dec(&_VTMS_transition_disable_for_one_count);
>>> 372: if (_VTMS_transition_disable_for_one_count == 0 || _is_SR) {
>>
>> Sorry I don't understand why this `_is_SR` check was removed. I admit I
>> can't really figure out what this field means anyway, but there is nothing
>> in the issue description that suggests this also needs changing - and it is
>> now different to `VTMS_transition_enable_for_all`.
>
> A JvmtiVTMSTransitionDisabler instance that is a "single disabler" only
> blocks other virtual threads trying to transition or
> JvmtiVTMSTransitionDisabler monopolists. Both of them will check for
> _VTMS_transition_disable_for_one_count (the JvmtiVTMSTransitionDisabler
> monopolist was missing that check) so just checking when that counter is zero
> is enough. In fact, for a "single disabler" _is_SR is always false so that
> check wasn't doing anything. Yes, this is not actually needed for the fix,
> but when looking at which condition we use to wait and which one to notify I
> caught this, sorry for not explaining that part.
>
> And looking closer at VTMS_transition_enable_for_all() now I see the check
> for _is_SR is not doing anything too, because if
> _VTMS_transition_disable_for_all_count was not zero after the decrement then
> this can't be a JvmtiVTMSTransitionDisabler monopolist, i.e _is_SR will be
> false. When a monopolist is running all other "disable all"
> JvmtiVTMSTransitionDisabler instances if any will be waiting in the first
> "while (_SR_mode)" loop in VTMS_transition_disable_for_all(), so
> _VTMS_transition_disable_for_all_count will be one through the monopolist
> run. So this should be an assert after the decrement: assert(!_is_SR ||
> _VTMS_transition_disable_for_all_count == 0, "").
Thanks for clarifying - I was puzzled by the way `is_SR` was being used.
-------------
PR: https://git.openjdk.org/jdk/pull/12956
