RFR: 8295976: GetThreadListStackTraces returns wrong state for blocked VirtualThread [v4]

Alex Menkov amenkov at openjdk.org
Tue Jun 6 22:39:09 UTC 2023


On Tue, 6 Jun 2023 21:37:25 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

>> The `GetThreadListStackTraces` returns `JVMTI_THREAD_STATE_RUNNABLE` for a VirtualThread blocked on a monitor when called for more than one thread. When called for a single VirtualThread it correctly returns a state that includes the `JVMTI_THREAD_STATE_BLOCKED_ON_MONITOR_ENTER` flag.
>> The `VM_GetThreadListStackTraces::doit` should call the `get_threadOop_and_JavaThread` instead of `cv_external_thread_to_JavaThread`. But the `get_threadOop_and_JavaThread` has a check for the current thread by comparing with the JavaThread::current() which does not work for a `VM_op`. Some refactoring of the `get_threadOop_and_JavaThread` was made to make it working for a `VM_op`.
>> Also, a minor bug in the `GetSingleStackTraceClosure::do_thread()` was discovered during testing.
>> 
>> The list of changes is:
>> - minor refactoring of the function`get_threadOop_and_JavaThread`: added an overloaded version of this function with the extra parameter `JavaThread* cur_thread`. It is called instead of `JvmtiExport::cv_external_thread_to_JavaThread` from the `VM_GetThreadListStackTraces::doit`.
>> - `GetSingleStackTraceClosure::do_thread()`: The use of `jt->threadObj()` is replaced with the `JNIHandles::resolve_external_guard(_jthread)`.
>>  - added new test to provide needed coverage: `test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest`
>>  
>> Testing:
>>  - ran new test: `test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest`
>>  - TBD: tiers 1-6 (all are good)
>
> Serguei Spitsyn has updated the pull request incrementally with one additional commit since the last revision:
> 
>   addressed new test related review comments

Marked as reviewed by amenkov (Reviewer).

test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest/ThreadListStackTracesTest.java line 29:

> 27:  * @summary GetThreadListStackTraces returns wrong state for blocked VirtualThread
> 28:  * @requires vm.continuations
> 29:  * @modules java.base/java.lang:+open

I think `@modules` it's not needed

test/hotspot/jtreg/serviceability/jvmti/vthread/ThreadListStackTracesTest/ThreadListStackTracesTest.java line 34:

> 32: 
> 33: import java.util.concurrent.locks.ReentrantLock;
> 34: import java.util.concurrent.*;

unused imports

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

PR Review: https://git.openjdk.org/jdk/pull/14326#pullrequestreview-1466246664
PR Review Comment: https://git.openjdk.org/jdk/pull/14326#discussion_r1220481289
PR Review Comment: https://git.openjdk.org/jdk/pull/14326#discussion_r1220480268


More information about the hotspot-dev mailing list