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