On Thu, 22 Dec 2022 21:55:02 GMT, Patricio Chilano Mateo <pchilanom...@openjdk.org> wrote:
>> Serguei Spitsyn has updated the pull request incrementally with one >> additional commit since the last revision: >> >> use owned_by_self vs is_locked in JvmtiVTMSTransition_lock asserts > > 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! ------------- PR: https://git.openjdk.org/jdk/pull/11690
