On Tue, 22 Jul 2025 05:47:37 GMT, David Holmes <[email protected]> wrote:
>> The `cv_internal_thread_to_JavaThread` method will now check if the thread
>> is a mounted virtual thread, and if so protect the carrier thread. User's of
>> the API that need to deal with virtual threads must still check the carrier
>> themselves as it could change asynchronously, but it is now guaranteed that
>> the carrier JavaThread returned via this method cannot terminate whilst
>> being used (the same as regular platform JavaThreads).
>>
>> It was noticed whilst updating the documentation that the `JvmtiExport`
>> variant `cv_oop_to_JavaThread` is unused and so can be removed.
>>
>> Testing:
>> - tiers 1-4
>>
>> Thanks
>
> David Holmes has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Move comment
Thumbs up on the changes. I think I only have a few nits.
src/hotspot/share/prims/jvmtiExport.cpp line 871:
> 869: //
> 870: jvmtiError
> 871: JvmtiExport::cv_oop_to_JavaThread(ThreadsList * t_list, oop thread_oop,
When did `JvmtiExport::cv_oop_to_JavaThread` become unused?
src/hotspot/share/runtime/threadSMR.cpp line 833:
> 831: return false;
> 832: } else {
> 833: // For virtual thread's we need to extract the carrier's
> JavaThread - if any.
nit typo: s/thread's/threads/
src/hotspot/share/runtime/threadSMR.hpp line 72:
> 70: // : // do stuff with 'jt'...
> 71: //
> 72: // A JavaThread* that is included in the ThreadsList that is held by
Why change just this location from `JavaThread *` to `JavaThread*`?
There are other places that you touched that still use `JavaThread *`.
src/hotspot/share/runtime/threadSMR.hpp line 310:
> 308: inline Iterator end();
> 309:
> 310: bool cv_internal_thread_to_JavaThread(jobject jthread, JavaThread**
> jt_pp, oop* thread_oop_p);
I wish I could remember why in the world I put a space before the `*` when I
did this code so very long ago...
-------------
Marked as reviewed by dcubed (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/26287#pullrequestreview-3056275005
PR Review Comment: https://git.openjdk.org/jdk/pull/26287#discussion_r2231715585
PR Review Comment: https://git.openjdk.org/jdk/pull/26287#discussion_r2231668135
PR Review Comment: https://git.openjdk.org/jdk/pull/26287#discussion_r2231710443
PR Review Comment: https://git.openjdk.org/jdk/pull/26287#discussion_r2231713403
