On Thu, 21 Dec 2023 12:23:35 GMT, Serguei Spitsyn <sspit...@openjdk.org> wrote:
>> I mean race between virtual thread state change and the thread stack switch >> (to/from carrier). >> Correct condition here is "dump the virtual thread if it was not dumped as >> mounted virtual thread". >> Most likely for RUNNABLE/PINNED/TIMED_PINNED we can be sure the thread is >> mounted, but we can get vthread in transition (PARKING, YIELDING, etc). >> Virtual threads may be in transition at safepoints (but they can't change >> mounted/unmounted state). >> So I think `is_vthread_mounted` can be implemented in 2 ways: >> 1) copy logic of JvmtiEnvBase::get_JavaThread_or_null: >> get JavaThread for java_lang_VirtualThread::carrier_thread(vt); >> if not null, check Continuation::is_continuation_mounted(java_thread, >> java_lang_VirtualThread::continuation(vt)) - this is to handle transition, >> when vthread is already unmounted, but carrierThread is not yet set to null; >> 2) check that java_lang_VirtualThread::continuation(vt) doesn't have >> non-empty chunk. >> AFAIU this is true for mounted vthreads. If we get it for unmounted vt, >> its stack trace of the thread is empty anyway, so it's ok to skip it (most >> likely it can happen only if thread state is NEW or TERMINATED, we already >> skip such vthreads). >> >> @AlanBateman could you please comment if my understanding is correct > >> I mean race between virtual thread state change and the thread stack switch >> (to/from carrier). > > I'm not sure if I understand you correctly or if we can call it a race. Alan > will correct me if I'm wrong. > You are talking about thread state change. At least, mount state transition > happens on the same JavaThread (it seems, you call it thread state switch). > Mount state transition can go over a safepoint. But it should not progress > while in a safepoint. David pointed out, "this was all happening at a global > safepoint". My understanding is this assumption is correct. Then your > approach `to identify that a virtual thread is mounted or not` should work in > general. The condition `java_lang_VirtualThread::carrier_thread(vt) != > nullptr` should indicate that the `vt` is mounted or is being in mount or > unmount transition. > If the `vt` is in mount or unmount transition then (it is a gray zone) the > way we identify mounted state should match the way we did it when dumped > mounted virtual threads. > It is done this way: `oop mounted_vt = thread->is_vthread_mounted() ? > thread->vthread() : nullptr;` > So, it seems any of yous suggestion should work here. Though, it would be > nice to simplify it a little if possible. Again, to be consistent, a `vt` in > mount state transition just have to be identified as mounted or unmounted in > both fragments in a similar way . Looks like "race" is wrong word here. There is no race between different threads, we just cannot rely on vt state or carrierThread value when the thread in mount/unmount transition. Sorry for the confusion. Serguei, thank you for the analysis. I agree, the code for mounted and unmounted vthreads should be consistent. For unmounted threads we have to get JavaThread of the carrier thread and if it's not null, check java_thread->is_vthread_mounted(). We don't need to check `is_continuation_mounted` as we are at safepoint. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/17134#discussion_r1434583304
