On Thu, 24 Jul 2025 09:44:20 GMT, Serguei Spitsyn <sspit...@openjdk.org> wrote:
>> David Holmes has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Move comment
>
> src/hotspot/share/runtime/threadSMR.cpp line 839:
>
>> 837: }
>> 838: if (java_thread == nullptr) {
>> 839: // Virtual thread was unmounted, or else carrier has now
>> terminated.
>
> Nit: If `java_thread` for a virtual thread is null then this thread is
> unmounted. The words about the terminated carrier thread are confusing.
>
> I have one concern here. Nothing protects this code from facing some
> unexpected conditions as this function is racy with mounting and unmounting
> of target virtual thread
> - this function can observe a `JavaThread` in `mount` or `unmount`
> transition where the thread identity of a `java.lang.Thread` associated with
> the `JavaThread` is not consistent (in a VTMS transition the thread identity
> might not match the stack trace)
> - nothing guaranties that the result returned by this function remains the
> same upon return as `mount`/`unmount` of the target virtual thread can be
> executed concurrently: a mounted virtual thread can be quickly unmounted or
> an unmounted virtual thread can be quickly mounted
>
> So, it seems that this function is going to work correctly if used for target
> platform threads only or if the `JavaThread*` pointer is not really needed.
> Otherwise, the association with the `JavaThread` needs to be rechecked and
> its stability somehow guaranteed with any form of scheduling suspension or a
> VTMS transition disabler.
The java_thread can be null because the carrier of a mounted vthread was not
contained in the ThreadsListHandle.
The only thing this function guarantees is that if the vthread appeared to have
a carrier and that carrier is protected by the TLH, then the caller can safely
interact with the carrier knowing it can't terminate - same as for regular
threads. The caller has to account for the potential async mounting/unmounting
of vthread - e.g. by handshaking the reported carrier and then confirming it
is still the carrier.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26287#discussion_r2228382680
