RFR: 8341273: JVMTI is not properly hiding some continuation related methods

Leonid Mesnik lmesnik at openjdk.org
Tue Oct 8 00:13:58 UTC 2024


On Mon, 7 Oct 2024 22:03:36 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

> This fixes a problem in the VTMS (Virtual Thread Mount State) transition frames hiding mechanism.
> Please, see a fix description in the first comment.
> 
> Testing:
>  - Verified with new test `vthread/CheckHiddenFrames`
>  - Mach5 tiers 1-6 are passed

Changes requested by lmesnik (Reviewer).

src/hotspot/share/prims/jvmtiEnvBase.cpp line 691:

> 689: 
> 690:   if (is_virtual || jt->is_in_VTMS_transition()) { // filter out pure continuations
> 691:     jvf = check_and_skip_hidden_frames(jt->is_in_VTMS_transition(), jvf);

Wouldn't be easier to split method `check_and_skip_hidden_frames` to skip_yeilds and skip_transition frames? 
BTW Also it is unclear shouldn't the `@Hidden` methods be skipped from all jvmti frame streams?

src/hotspot/share/prims/jvmtiEnvBase.cpp line 2009:

> 2007:   bool is_virtual = java_lang_VirtualThread::is_instance(thread_oop);
> 2008: 
> 2009:   // Target can be virtual or platform thread.

Can you please explain in comment why is it needed to disable all threads for platform target thread.

test/hotspot/jtreg/serviceability/jvmti/vthread/CheckHiddenFrames/CheckHiddenFrames.java line 25:

> 23: 
> 24: /*
> 25:  * @test id=virtual

Having 'id=virtual' not needed and might confuse people. They expect to have other test variations for platform.

test/hotspot/jtreg/serviceability/jvmti/vthread/CheckHiddenFrames/CheckHiddenFrames.java line 32:

> 30: 
> 31: public class CheckHiddenFrames {
> 32:     private static final String AGENT_LIB = "CheckHiddenFrames";

It is not used?

test/hotspot/jtreg/serviceability/jvmti/vthread/CheckHiddenFrames/CheckHiddenFrames.java line 43:

> 41: 
> 42:     public static void main(String[] args) throws Exception {
> 43:         Thread thread = Thread.ofVirtual().unstarted(CheckHiddenFrames::test);

You can use 
startVirtualThread
to save one line.

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

PR Review: https://git.openjdk.org/jdk/pull/21397#pullrequestreview-2353060666
PR Review Comment: https://git.openjdk.org/jdk/pull/21397#discussion_r1791023030
PR Review Comment: https://git.openjdk.org/jdk/pull/21397#discussion_r1791008060
PR Review Comment: https://git.openjdk.org/jdk/pull/21397#discussion_r1790967726
PR Review Comment: https://git.openjdk.org/jdk/pull/21397#discussion_r1790968113
PR Review Comment: https://git.openjdk.org/jdk/pull/21397#discussion_r1790966876


More information about the core-libs-dev mailing list