From ecbdd3405a1d46f555deb82098e1865b44601a1f Mon Sep 17 00:00:00 2001 From: Alex Menkov Date: Wed, 13 Aug 2025 18:24:56 +0000 Subject: [PATCH] 8361103: java_lang_Thread::async_get_stack_trace does not properly protect JavaThread Reviewed-by: sspitsyn, dholmes --- src/hotspot/share/classfile/javaClasses.cpp | 53 ++++++++++----------- src/hotspot/share/classfile/javaClasses.hpp | 2 +- src/hotspot/share/prims/jvm.cpp | 2 +- 3 files changed, 27 insertions(+), 30 deletions(-) diff --git a/src/hotspot/share/classfile/javaClasses.cpp b/src/hotspot/share/classfile/javaClasses.cpp index 28bebee7d9a..5c41ea789b5 100644 --- a/src/hotspot/share/classfile/javaClasses.cpp +++ b/src/hotspot/share/classfile/javaClasses.cpp @@ -1899,33 +1899,29 @@ oop java_lang_Thread::park_blocker(oop java_thread) { return java_thread->obj_field_access(_park_blocker_offset); } -oop java_lang_Thread::async_get_stack_trace(oop java_thread, TRAPS) { - ThreadsListHandle tlh(JavaThread::current()); - JavaThread* thread; - bool is_virtual = java_lang_VirtualThread::is_instance(java_thread); - if (is_virtual) { - oop carrier_thread = java_lang_VirtualThread::carrier_thread(java_thread); - if (carrier_thread == nullptr) { - return nullptr; - } - thread = java_lang_Thread::thread(carrier_thread); - } else { - thread = java_lang_Thread::thread(java_thread); - } - if (thread == nullptr) { +// Obtain stack trace for platform or mounted virtual thread. +// If jthread is a virtual thread and it has been unmounted (or remounted to different carrier) the method returns null. +// The caller (java.lang.VirtualThread) handles returned nulls via retry. +oop java_lang_Thread::async_get_stack_trace(jobject jthread, TRAPS) { + ThreadsListHandle tlh(THREAD); + JavaThread* java_thread = nullptr; + oop thread_oop; + + bool has_java_thread = tlh.cv_internal_thread_to_JavaThread(jthread, &java_thread, &thread_oop); + if (!has_java_thread) { return nullptr; } class GetStackTraceHandshakeClosure : public HandshakeClosure { public: - const Handle _java_thread; + const Handle _thread_h; int _depth; bool _retry_handshake; GrowableArray* _methods; GrowableArray* _bcis; - GetStackTraceHandshakeClosure(Handle java_thread) : - HandshakeClosure("GetStackTraceHandshakeClosure"), _java_thread(java_thread), _depth(0), _retry_handshake(false), + GetStackTraceHandshakeClosure(Handle thread_h) : + HandshakeClosure("GetStackTraceHandshakeClosure"), _thread_h(thread_h), _depth(0), _retry_handshake(false), _methods(nullptr), _bcis(nullptr) { } ~GetStackTraceHandshakeClosure() { @@ -1947,21 +1943,22 @@ oop java_lang_Thread::async_get_stack_trace(oop java_thread, TRAPS) { return; } - JavaThread* thread = JavaThread::cast(th); + JavaThread* java_thread = JavaThread::cast(th); - if (!thread->has_last_Java_frame()) { + if (!java_thread->has_last_Java_frame()) { return; } bool carrier = false; - if (java_lang_VirtualThread::is_instance(_java_thread())) { - // if (thread->vthread() != _java_thread()) // We might be inside a System.executeOnCarrierThread - const ContinuationEntry* ce = thread->vthread_continuation(); - if (ce == nullptr || ce->cont_oop(thread) != java_lang_VirtualThread::continuation(_java_thread())) { - return; // not mounted + if (java_lang_VirtualThread::is_instance(_thread_h())) { + // Ensure _thread_h is still mounted to java_thread. + const ContinuationEntry* ce = java_thread->vthread_continuation(); + if (ce == nullptr || ce->cont_oop(java_thread) != java_lang_VirtualThread::continuation(_thread_h())) { + // Target thread has been unmounted. + return; } } else { - carrier = (thread->vthread_continuation() != nullptr); + carrier = (java_thread->vthread_continuation() != nullptr); } const int max_depth = MaxJavaStackTraceDepth; @@ -1973,7 +1970,7 @@ oop java_lang_Thread::async_get_stack_trace(oop java_thread, TRAPS) { _bcis = new (mtInternal) GrowableArray(init_length, mtInternal); int total_count = 0; - for (vframeStream vfst(thread, false, false, carrier); // we don't process frames as we don't care about oops + for (vframeStream vfst(java_thread, false, false, carrier); // we don't process frames as we don't care about oops !vfst.at_end() && (max_depth == 0 || max_depth != total_count); vfst.next()) { @@ -1994,9 +1991,9 @@ oop java_lang_Thread::async_get_stack_trace(oop java_thread, TRAPS) { // Handshake with target ResourceMark rm(THREAD); HandleMark hm(THREAD); - GetStackTraceHandshakeClosure gsthc(Handle(THREAD, java_thread)); + GetStackTraceHandshakeClosure gsthc(Handle(THREAD, thread_oop)); do { - Handshake::execute(&gsthc, &tlh, thread); + Handshake::execute(&gsthc, &tlh, java_thread); } while (gsthc.read_reset_retry()); // Stop if no stack trace is found. diff --git a/src/hotspot/share/classfile/javaClasses.hpp b/src/hotspot/share/classfile/javaClasses.hpp index faa325d55dd..70fa519f0f0 100644 --- a/src/hotspot/share/classfile/javaClasses.hpp +++ b/src/hotspot/share/classfile/javaClasses.hpp @@ -463,7 +463,7 @@ class java_lang_Thread : AllStatic { static const char* thread_status_name(oop java_thread_oop); // Fill in current stack trace, can cause GC - static oop async_get_stack_trace(oop java_thread, TRAPS); + static oop async_get_stack_trace(jobject jthread, TRAPS); JFR_ONLY(static u2 jfr_epoch(oop java_thread);) JFR_ONLY(static void set_jfr_epoch(oop java_thread, u2 epoch);) diff --git a/src/hotspot/share/prims/jvm.cpp b/src/hotspot/share/prims/jvm.cpp index 005776b00c2..098bd729c0b 100644 --- a/src/hotspot/share/prims/jvm.cpp +++ b/src/hotspot/share/prims/jvm.cpp @@ -2945,7 +2945,7 @@ JVM_ENTRY(jboolean, JVM_HoldsLock(JNIEnv* env, jclass threadClass, jobject obj)) JVM_END JVM_ENTRY(jobject, JVM_GetStackTrace(JNIEnv *env, jobject jthread)) - oop trace = java_lang_Thread::async_get_stack_trace(JNIHandles::resolve(jthread), THREAD); + oop trace = java_lang_Thread::async_get_stack_trace(jthread, THREAD); return JNIHandles::make_local(THREAD, trace); JVM_END