RFR: 8298853: JvmtiVTMSTransitionDisabler should support disabling one virtual thread transitions [v4]

[David Holmes]

On Fri, 23 Dec 2022 00:26:02 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

>> src/hotspot/share/prims/jvmtiThreadState.cpp line 315:
>> 
>>> 313:     ml.wait(10); // wait while the virtual thread is in transition
>>> 314:   }
>>> 315:   java_lang_Thread::inc_VTMS_transition_disable_count(vth());
>> 
>> I think this sequence coupled with how start_VTMS_transition() works can have races. So the check vstate->is_in_VTMS_transition() could return false, but before we execute inc_VTMS_transition_disable_count() the target could have already called and returned from start_VTMS_transition(). 
>> Just to test this I added a nanosleep() call in between those two, and an assert before returning from JvmtiVTMSTransitionDisabler::JvmtiVTMSTransitionDisabler(jthread thread) that the target is not in the middle of a transition. Running the serviceability/jvmti/vthread/ tests gives me some crashes in that assert. Test patch: https://github.com/pchilano/jdk/commit/9bc22771bf324f69bc4f5a9786f1b21937aab3c7
>> 
>> If I compare with how the "disable all" case works, in there, the target transitioning sets a variable(_VTMS_transition_count) and then reads another one(_VTMS_transition_disable_for_all_count) set by the transition disabler. The transition disabler does the same but with the variables reversed, i.e. sets _VTMS_transition_disable_for_all_count and then reads _VTMS_transition_count. This correctly makes sure that if we read the value that allow us to return from the method then we have already written the flag that will stop the other thread. But in this "single" case I don't see the same pattern.
>
> I see the problem. Good catch, thanks!

Yep my bad in earlier review. The one case needs the same synchronization/coordination as the all case.

-------------

PR: https://git.openjdk.org/jdk/pull/11690