From b9d999182cd57d188a0c8eae862f3119a2d42c75 Mon Sep 17 00:00:00 2001 From: Serguei Spitsyn Date: Fri, 25 Oct 2024 20:49:34 +0000 Subject: [PATCH] review: introduce new annotation @JvmtiHideEvents and use it in VirtualThread/Continuation classes to disallow FramePop requests --- .../share/classfile/classFileParser.cpp | 8 ++++ src/hotspot/share/classfile/vmSymbols.hpp | 1 + src/hotspot/share/oops/constMethodFlags.hpp | 1 + src/hotspot/share/oops/method.hpp | 3 ++ src/hotspot/share/prims/jvmtiEnvBase.cpp | 40 +++++++++---------- src/hotspot/share/prims/jvmtiEnvBase.hpp | 1 - src/hotspot/share/prims/jvmtiThreadState.cpp | 13 +++--- .../classes/java/lang/VirtualThread.java | 3 +- .../classes/jdk/internal/vm/Continuation.java | 5 +++ .../vm/annotation/JvmtiHideEvents.java | 40 +++++++++++++++++++ .../vm/annotation/JvmtiMountTransition.java | 3 +- 11 files changed, 87 insertions(+), 31 deletions(-) create mode 100644 src/java.base/share/classes/jdk/internal/vm/annotation/JvmtiHideEvents.java diff --git a/src/hotspot/share/classfile/classFileParser.cpp b/src/hotspot/share/classfile/classFileParser.cpp index 0fb8d54233c26..66dec61946888 100644 --- a/src/hotspot/share/classfile/classFileParser.cpp +++ b/src/hotspot/share/classfile/classFileParser.cpp @@ -943,6 +943,7 @@ class AnnotationCollector : public ResourceObj{ _method_ForceInline, _method_DontInline, _method_ChangesCurrentThread, + _method_JvmtiHideEvents, _method_JvmtiMountTransition, _method_InjectedProfile, _method_LambdaForm_Compiled, @@ -1847,6 +1848,11 @@ AnnotationCollector::annotation_index(const ClassLoaderData* loader_data, if (!privileged) break; // only allow in privileged code return _method_ChangesCurrentThread; } + case VM_SYMBOL_ENUM_NAME(jdk_internal_vm_annotation_JvmtiHideEvents_signature): { + if (_location != _in_method) break; // only allow for methods + if (!privileged) break; // only allow in privileged code + return _method_JvmtiHideEvents; + } case VM_SYMBOL_ENUM_NAME(jdk_internal_vm_annotation_JvmtiMountTransition_signature): { if (_location != _in_method) break; // only allow for methods if (!privileged) break; // only allow in privileged code @@ -1934,6 +1940,8 @@ void MethodAnnotationCollector::apply_to(const methodHandle& m) { m->set_dont_inline(); if (has_annotation(_method_ChangesCurrentThread)) m->set_changes_current_thread(); + if (has_annotation(_method_JvmtiHideEvents)) + m->set_jvmti_hide_events(); if (has_annotation(_method_JvmtiMountTransition)) m->set_jvmti_mount_transition(); if (has_annotation(_method_InjectedProfile)) diff --git a/src/hotspot/share/classfile/vmSymbols.hpp b/src/hotspot/share/classfile/vmSymbols.hpp index 58e1551e20ca0..014a8a00c7b0d 100644 --- a/src/hotspot/share/classfile/vmSymbols.hpp +++ b/src/hotspot/share/classfile/vmSymbols.hpp @@ -306,6 +306,7 @@ class SerializeClosure; template(jdk_internal_vm_annotation_Stable_signature, "Ljdk/internal/vm/annotation/Stable;") \ \ template(jdk_internal_vm_annotation_ChangesCurrentThread_signature, "Ljdk/internal/vm/annotation/ChangesCurrentThread;") \ + template(jdk_internal_vm_annotation_JvmtiHideEvents_signature, "Ljdk/internal/vm/annotation/JvmtiHideEvents;") \ template(jdk_internal_vm_annotation_JvmtiMountTransition_signature, "Ljdk/internal/vm/annotation/JvmtiMountTransition;") \ \ /* Support for JSR 292 & invokedynamic (JDK 1.7 and above) */ \ diff --git a/src/hotspot/share/oops/constMethodFlags.hpp b/src/hotspot/share/oops/constMethodFlags.hpp index 236f25ea746fa..031ebe44a9654 100644 --- a/src/hotspot/share/oops/constMethodFlags.hpp +++ b/src/hotspot/share/oops/constMethodFlags.hpp @@ -60,6 +60,7 @@ class ConstMethodFlags { flag(jvmti_mount_transition , 1 << 18) \ flag(deprecated , 1 << 19) \ flag(deprecated_for_removal , 1 << 20) \ + flag(jvmti_hide_events , 1 << 21) \ /* end of list */ #define CM_FLAGS_ENUM_NAME(name, value) _misc_##name = value, diff --git a/src/hotspot/share/oops/method.hpp b/src/hotspot/share/oops/method.hpp index 6ffaebcdfdab2..d42089d3a5cc9 100644 --- a/src/hotspot/share/oops/method.hpp +++ b/src/hotspot/share/oops/method.hpp @@ -746,6 +746,9 @@ class Method : public Metadata { bool changes_current_thread() const { return constMethod()->changes_current_thread(); } void set_changes_current_thread() { constMethod()->set_changes_current_thread(); } + bool jvmti_hide_events() const { return constMethod()->jvmti_hide_events(); } + void set_jvmti_hide_events() { constMethod()->set_jvmti_hide_events(); } + bool jvmti_mount_transition() const { return constMethod()->jvmti_mount_transition(); } void set_jvmti_mount_transition() { constMethod()->set_jvmti_mount_transition(); } diff --git a/src/hotspot/share/prims/jvmtiEnvBase.cpp b/src/hotspot/share/prims/jvmtiEnvBase.cpp index d939edb5d3495..c28afbb1c51d0 100644 --- a/src/hotspot/share/prims/jvmtiEnvBase.cpp +++ b/src/hotspot/share/prims/jvmtiEnvBase.cpp @@ -651,22 +651,33 @@ JavaThread* JvmtiEnvBase::get_JavaThread_or_null(oop vthread) { return Continuation::is_continuation_mounted(java_thread, cont) ? java_thread : nullptr; } -// An unmounted vthread always has the yield0() and yield() frames we need to hide. +// An unmounted vthread may have an empty stack. +// Otherwise, it always has the yield0() and yield() frames we need to hide. +// The methods yield0() and yield() are annotated with the @JvmtiHideEvents. javaVFrame* JvmtiEnvBase::skip_yield_frames_for_unmounted_vthread(javaVFrame* jvf) { - if (jvf == nullptr) { // check for safety - return jvf; + if (jvf == nullptr) { + return jvf; // empty stack is possible } + assert(jvf->method()->jvmti_hide_events(), "sanity check"); + assert(jvf->method()->method_holder() == vmClasses::Continuation_klass(), "expected Continuation class"); jvf = jvf->java_sender(); // skip yield0 frame - assert(jvf != nullptr, "sanity check"); + + assert(jvf != nullptr && jvf->method()->jvmti_hide_events(), "sanity check"); + assert(jvf->method()->method_holder() == vmClasses::Continuation_klass(), "expected Continuation class"); jvf = jvf->java_sender(); // skip yield frame return jvf; } +// A thread may have an empty stack. +// Otherwise, some top frames may heed to be hidden. +// Two cases are processed below: +// - top frame is annotated with @JvmtiMountTransition: just skip top frames with annotated methods +// - JavaThread is in VTMS transition: skip top frames until a frame annotated with @ChangesCurrentThread is found javaVFrame* JvmtiEnvBase::check_and_skip_hidden_frames(bool is_in_VTMS_transition, javaVFrame* jvf) { if (jvf == nullptr) { - return jvf; + return jvf; // empty stack is possible } if (jvf->method()->jvmti_mount_transition()) { // Skip frames annotated with @JvmtiMountTransition. @@ -686,18 +697,6 @@ JvmtiEnvBase::check_and_skip_hidden_frames(JavaThread* jt, javaVFrame* jvf) { return jvf; } -javaVFrame* -JvmtiEnvBase::check_and_skip_hidden_frames(oop vthread, javaVFrame* jvf) { - assert(java_lang_VirtualThread::is_instance(vthread), "sanity check"); - if (java_lang_Thread::is_in_VTMS_transition(vthread)) { - jvf = check_and_skip_hidden_frames(true, jvf); - } else { - // if vthread is not in a VTMS transition then it is unmounted - jvf = skip_yield_frames_for_unmounted_vthread(jvf); - } - return jvf; -} - javaVFrame* JvmtiEnvBase::get_vthread_jvf(oop vthread) { assert(java_lang_VirtualThread::state(vthread) != java_lang_VirtualThread::NEW, "sanity check"); @@ -722,7 +721,7 @@ JvmtiEnvBase::get_vthread_jvf(oop vthread) { } else { vframeStream vfs(cont); jvf = vfs.at_end() ? nullptr : vfs.asJavaVFrame(); - jvf = check_and_skip_hidden_frames(vthread, jvf); + jvf = skip_yield_frames_for_unmounted_vthread(jvf); } return jvf; } @@ -1340,8 +1339,9 @@ JvmtiEnvBase::set_frame_pop(JvmtiThreadState* state, javaVFrame* jvf, jint depth if (jvf == nullptr) { return JVMTI_ERROR_NO_MORE_FRAMES; } - if (jvf->method()->is_native() || (depth == 0 && state->top_frame_is_exiting()) || - (state->is_virtual() && (jvf->is_vthread_entry() || jvf->method()->jvmti_mount_transition()))) { + if (jvf->method()->is_native() || + (depth == 0 && state->top_frame_is_exiting()) || + (state->is_virtual() && jvf->method()->jvmti_hide_events())) { return JVMTI_ERROR_OPAQUE_FRAME; } assert(jvf->frame_pointer() != nullptr, "frame pointer mustn't be null"); diff --git a/src/hotspot/share/prims/jvmtiEnvBase.hpp b/src/hotspot/share/prims/jvmtiEnvBase.hpp index 590720f18f766..e8769d423c531 100644 --- a/src/hotspot/share/prims/jvmtiEnvBase.hpp +++ b/src/hotspot/share/prims/jvmtiEnvBase.hpp @@ -369,7 +369,6 @@ class JvmtiEnvBase : public CHeapObj { static javaVFrame* skip_yield_frames_for_unmounted_vthread(javaVFrame* jvf); static javaVFrame* check_and_skip_hidden_frames(bool is_in_VTMS_transition, javaVFrame* jvf); static javaVFrame* check_and_skip_hidden_frames(JavaThread* jt, javaVFrame* jvf); - static javaVFrame* check_and_skip_hidden_frames(oop vthread, javaVFrame* jvf); // check if virtual thread is not terminated (alive) static bool is_vthread_alive(oop vt); diff --git a/src/hotspot/share/prims/jvmtiThreadState.cpp b/src/hotspot/share/prims/jvmtiThreadState.cpp index 0fc393071dd37..743f112b9b6b8 100644 --- a/src/hotspot/share/prims/jvmtiThreadState.cpp +++ b/src/hotspot/share/prims/jvmtiThreadState.cpp @@ -253,7 +253,7 @@ JvmtiVTMSTransitionDisabler::print_info() { #endif // disable VTMS transitions for one virtual thread -// no-op if thread is non-null and not a virtual thread +// disable VTMS transitions for all threads if thread is nullptr or a platform thread JvmtiVTMSTransitionDisabler::JvmtiVTMSTransitionDisabler(jthread thread) : _is_SR(false), _thread(thread) { @@ -269,7 +269,7 @@ JvmtiVTMSTransitionDisabler::JvmtiVTMSTransitionDisabler(jthread thread) oop thread_oop = JNIHandles::resolve_external_guard(thread); // Target can be virtual or platform thread. - // If traget is a platform thread then we have to disable VTMS transitions for all threads. + // If target is a platform thread then we have to disable VTMS transitions for all threads. // It is by several reasons: // - carrier threads can mount virtual threads which may cause incorrect behavior // - there is no mechanism to disable transitions for a specific carrier thread yet @@ -327,9 +327,8 @@ JvmtiVTMSTransitionDisabler::VTMS_transition_disable_for_one() { JavaThread* thread = JavaThread::current(); HandleMark hm(thread); Handle vth = Handle(thread, JNIHandles::resolve_external_guard(_thread)); - if (!java_lang_VirtualThread::is_instance(vth())) { - return; // no-op if _thread is not a virtual thread - } + assert(java_lang_VirtualThread::is_instance(vth()), "sanity check"); + MonitorLocker ml(JvmtiVTMSTransition_lock); while (_SR_mode) { // suspender or resumer is a JvmtiVTMSTransitionDisabler monopolist @@ -479,7 +478,7 @@ JvmtiVTMSTransitionDisabler::start_VTMS_transition(jthread vthread, bool is_moun JvmtiVTSuspender::is_vthread_suspended(thread_id) ) { // Block while transitions are disabled or there are suspend requests. - if (ml.wait(10)) { + if (ml.wait(200)) { attempts--; } DEBUG_ONLY(if (attempts == 0) break;) @@ -536,7 +535,7 @@ JvmtiVTMSTransitionDisabler::finish_VTMS_transition(jthread vthread, bool is_mou (is_mount && JvmtiVTSuspender::is_vthread_suspended(thread_id)) ) { // Block while there are suspend requests. - if (ml.wait(10)) { + if (ml.wait(200)) { attempts--; } DEBUG_ONLY(if (attempts == 0) break;) diff --git a/src/java.base/share/classes/java/lang/VirtualThread.java b/src/java.base/share/classes/java/lang/VirtualThread.java index 9eb788a32dafe..8f389906f2a75 100644 --- a/src/java.base/share/classes/java/lang/VirtualThread.java +++ b/src/java.base/share/classes/java/lang/VirtualThread.java @@ -56,6 +56,7 @@ import jdk.internal.vm.annotation.ChangesCurrentThread; import jdk.internal.vm.annotation.Hidden; import jdk.internal.vm.annotation.IntrinsicCandidate; +import jdk.internal.vm.annotation.JvmtiHideEvents; import jdk.internal.vm.annotation.JvmtiMountTransition; import jdk.internal.vm.annotation.ReservedStackAccess; import sun.nio.ch.Interruptible; @@ -213,7 +214,7 @@ protected void onPinned(Continuation.Pinned reason) { private static Runnable wrap(VirtualThread vthread, Runnable task) { return new Runnable() { @Hidden - @JvmtiMountTransition + @JvmtiHideEvents public void run() { vthread.notifyJvmtiStart(); // notify JVMTI try { diff --git a/src/java.base/share/classes/jdk/internal/vm/Continuation.java b/src/java.base/share/classes/jdk/internal/vm/Continuation.java index 99d0c62aaec8d..b863def8e6a78 100644 --- a/src/java.base/share/classes/jdk/internal/vm/Continuation.java +++ b/src/java.base/share/classes/jdk/internal/vm/Continuation.java @@ -36,6 +36,7 @@ import jdk.internal.access.JavaLangAccess; import jdk.internal.access.SharedSecrets; import jdk.internal.vm.annotation.Hidden; +import jdk.internal.vm.annotation.JvmtiHideEvents; /** * A one-shot delimited continuation. @@ -305,6 +306,7 @@ private void finish() { @Hidden @DontInline @IntrinsicCandidate + @JvmtiHideEvents private static void enter(Continuation c, boolean isContinue) { // This method runs in the "entry frame". // A yield jumps to this method's caller as if returning from this method. @@ -316,6 +318,7 @@ private static void enter(Continuation c, boolean isContinue) { } @Hidden + @JvmtiHideEvents private void enter0() { target.run(); } @@ -340,6 +343,7 @@ private boolean isEmpty() { * @throws IllegalStateException if not currently in the given {@code scope}, */ @Hidden + @JvmtiHideEvents public static boolean yield(ContinuationScope scope) { Continuation cont = JLA.getContinuation(currentCarrierThread()); Continuation c; @@ -352,6 +356,7 @@ public static boolean yield(ContinuationScope scope) { } @Hidden + @JvmtiHideEvents private boolean yield0(ContinuationScope scope, Continuation child) { preempted = false; diff --git a/src/java.base/share/classes/jdk/internal/vm/annotation/JvmtiHideEvents.java b/src/java.base/share/classes/jdk/internal/vm/annotation/JvmtiHideEvents.java new file mode 100644 index 0000000000000..bfb4ffa5290f2 --- /dev/null +++ b/src/java.base/share/classes/jdk/internal/vm/annotation/JvmtiHideEvents.java @@ -0,0 +1,40 @@ +/* + * Copyright (c) 2021, 2022, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +package jdk.internal.vm.annotation; + +import java.lang.annotation.*; + +/** + * A method may be annotated with JvmtiHideEvents to hint it is not + * desirable to sent JVMTI events in context of the annotated method. + * + * @implNote + * This annotation is only used for some VirtualThread and Continuation methods. + */ +@Target({ElementType.METHOD}) +@Retention(RetentionPolicy.RUNTIME) +public @interface JvmtiHideEvents { +} diff --git a/src/java.base/share/classes/jdk/internal/vm/annotation/JvmtiMountTransition.java b/src/java.base/share/classes/jdk/internal/vm/annotation/JvmtiMountTransition.java index 9ccb3241ae460..504e83803865c 100644 --- a/src/java.base/share/classes/jdk/internal/vm/annotation/JvmtiMountTransition.java +++ b/src/java.base/share/classes/jdk/internal/vm/annotation/JvmtiMountTransition.java @@ -33,10 +33,9 @@ * Normally, a method is annotated with @JvmtiMountTransition if it starts * or ends Virtual Thread Mount State (VTMS) transition, so the thread * identity is undefined or different at method entry and exit. - * Specifically, it is used for all the VirtualThread notifyJvmti* methods. * * @implNote - * This annotation is only used for some VirtualThread and Continuation methods. + * This annotation is only used for the VirtualThread notifyJvmti* methods. */ @Target({ElementType.METHOD}) @Retention(RetentionPolicy.RUNTIME)