On Tue, 6 Jun 2023 21:37:25 GMT, Serguei Spitsyn <sspit...@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
