RFR: 8316397: StackTrace/Suspended/GetStackTraceSuspendedStressTest.java failed with: SingleStep event is NOT expected [v4]
Serguei Spitsyn
sspitsyn at openjdk.org
Tue Feb 25 10:57:53 UTC 2025
On Tue, 25 Feb 2025 07:34:24 GMT, David Holmes <dholmes at openjdk.org> wrote:
>>> But I would expect many blocked transitions to relate to something that will causes events to be posted, so do we end up effectively doing the suspension check when coming out of the blocked state?
>>>
>> Only for the ones in ObjectMonitor, JvmtiRawMonitor and JavaThread::wait_for_object_deoptimization(). In these cases we know there are no VM monitors held though.
>>
>>> Or should we simply be passing allow_suspend=true into more ThreadBlockInVM transitions?
>>>
>> I think this would be harder to get right. First because these ThreadBlockInVM could be anywhere along the path where we post an event. But also because at those points the thread could be holding VM monitors so we cannot suspend. Here where we post the event we are already checking no VM monitors are held.
>>
>>> And again what is the call stack for this?
>>>
>> For single step event the call is coming from InterpreterRuntime::at_safepoint():
>>
>>
>> Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code)
>> V [libjvm.so+0x1212523] JvmtiExport::get_jvmti_thread_state(JavaThread*)+0xd3 (jvmtiExport.cpp:425)
>> V [libjvm.so+0x121de66] JvmtiExport::at_single_stepping_point(JavaThread*, Method*, unsigned char*)+0x66 (jvmtiExport.cpp:1332)
>> V [libjvm.so+0xed8398] InterpreterRuntime::at_safepoint(JavaThread*)+0x118 (interpreterRuntime.cpp:1165)
>>
>>
>> But note this same issue can happen with other events, the test is just checking for single step.
>
>> But note this same issue can happen with other events, the test is just checking for single step.
>
> Does that mean we need the suspension check (and suspend) at other locations?
>
> I am looking at `JRT_ENTRY` and it uses `ThreadInVMfromJava` - so why are we not checking for suspension in that transition somewhere, or else somewhere directly in the JRT_ENTRY? I guess maybe not all JRT_ENTRY points are safe for suspension? But then how do we know all the callers of `get_jvmti_thread_state` are safe for suspension?
> What does the actual call-stack look like when we get here? We are obviously missing a suspension check for the current thread, but I still feel that should be higher-up the call-stack.
As Patricio pointed out (thanks, Patricio), with this particular test `GetStackTraceSuspendedStressTest.java` the `JvmtiExport::get_jvmti_thread_state` is called from `JvmtiExport::at_single_stepping_point()` which is called from `InterpreterRuntime::at_safepoint()`. There are no other points before to place the `ThreadBlockInVM`. It can be different for other events.
As an experiment I've added the following assert at the beginning of the `JvmtiExport::get_jvmti_thread_state()` and ran the mach5 tiers 1-6:
+ assert(!thread->owns_locks(), "all locks must be released before suspension");
and got many failures of the following tests:
applications/renaissance/RenaissanceStressTest.java
applications/runthese/RunThese30M.java
applications/skynet/SkyNet.java
serviceability/jvmti/DynamicCodeGenerated/DynamicCodeGeneratedTest.java
vmTestbase/nsk/jvmti/DynamicCodeGenerated/dyncodgen001/TestDescription.java
vmTestbase/nsk/jvmti/scenarios/events/EM04/em04t001/TestDescription.java
The assert was only triggered in the context of `post_dynamic_code_generated_while_holding_locks()` which make a direct call to `JvmtiExport::get_jvmti_thread_state()`.
A typical stack trace is:
Current thread (0x00007ff468008290): JavaThread "ForkJoinPool-1-worker-4" daemon [_thread_in_vm, id=437394, stack(0x00007ff4eb6ea000,0x00007ff4eb7ea000) (1024K)]
Stack: [0x00007ff4eb6ea000,0x00007ff4eb7ea000], sp=0x00007ff4eb7e5190, free space=1004k
Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code)
V [libjvm.so+0x1223114] JvmtiExport::get_jvmti_thread_state(JavaThread*)+0x94 (jvmtiExport.cpp:423)
V [libjvm.so+0x122628c] JvmtiExport::post_dynamic_code_generated_while_holding_locks(char const*, unsigned char*, unsigned char*)+0x3c (jvmtiExport.cpp:2636)
V [libjvm.so+0x19ad47e] VtableStubs::find_stub(bool, int)+0x1ce (vtableStubs.cpp:238)
V [libjvm.so+0xa81b0b] CompiledIC::set_to_megamorphic(CallInfo*)+0x4b (vtableStubs.hpp:107)
V [libjvm.so+0x17005a4] SharedRuntime::handle_ic_miss_helper(JavaThread*)+0x2d4 (sharedRuntime.cpp:1637)
V [libjvm.so+0x1700a08] SharedRuntime::handle_wrong_method_ic_miss(JavaThread*)+0xf8 (sharedRuntime.cpp:1441)
v ~RuntimeStub::Shared Runtime ic_miss_blob 0x00007ff50be5f449
J 378 c1 java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(Ljava/util/concurrent/ForkJoinTask;I)V java.base at 25-internal (18 bytes) @ 0x00007ff50485c274 [0x00007ff50485c040+0x0000000000000234]
j java.util.concurrent.ForkJoinPool.runWorker(Ljava/util/concurrent/ForkJoinPool$WorkQueue;)V+362 java.base at 25-internal
j java.util.concurrent.ForkJoinWorkerThread.run()V+31 java.base at 25-internal
v ~StubRoutines::call_stub 0x00007ff50bd48d01
V [libjvm.so+0xf06d9c] JavaCalls::call_helper(JavaValue*, methodHandle const&, JavaCallArguments*, JavaThread*)+0x4ac (javaCalls.cpp:415)
V [libjvm.so+0xf07483] JavaCalls::call_virtual(JavaValue*, Klass*, Symbol*, Symbol*, JavaCallArguments*, JavaThread*)+0x2f3 (javaCalls.cpp:323)
V [libjvm.so+0xf07a86] JavaCalls::call_virtual(JavaValue*, Handle, Klass*, Symbol*, Symbol*, JavaThread*)+0x76 (javaCalls.cpp:185)
V [libjvm.so+0x107cc33] thread_entry(JavaThread*, JavaThread*)+0x93 (jvm.cpp:2756)
V [libjvm.so+0xf417de] JavaThread::thread_main_inner()+0xee (javaThread.cpp:776)
V [libjvm.so+0x1896416] Thread::call_run()+0xb6 (thread.cpp:231)
V [libjvm.so+0x156e5a8] thread_native_entry(Thread*)+0x128 (os_linux.cpp:877)
The following fix fixed the issue:
git diff
diff --git a/src/hotspot/share/prims/jvmtiExport.cpp b/src/hotspot/share/prims/jvmtiExport.cpp
index 377dece1b1c..eda99e57b5d 100644
--- a/src/hotspot/share/prims/jvmtiExport.cpp
+++ b/src/hotspot/share/prims/jvmtiExport.cpp
@@ -418,11 +418,12 @@ JvmtiExport::get_jvmti_interface(JavaVM *jvm, void **penv, jint version) {
}
JvmtiThreadState*
-JvmtiExport::get_jvmti_thread_state(JavaThread *thread) {
+JvmtiExport::get_jvmti_thread_state(JavaThread *thread, bool allow_suspend) {
assert(thread == JavaThread::current(), "must be current thread");
+ assert(!allow_suspend || !thread->owns_locks(), "all locks must be released before suspension");
if (thread->is_vthread_mounted() && thread->jvmti_thread_state() == nullptr) {
JvmtiEventController::thread_started(thread);
- if (thread->is_suspended()) {
+ if (allow_suspend && thread->is_suspended()) {
// suspend here if there is a suspend request
ThreadBlockInVM tbivm(thread, true /* allow suspend */);
}
@@ -2632,7 +2633,7 @@ void JvmtiExport::post_dynamic_code_generated_while_holding_locks(const char* na
// jvmti thread state.
// The collector and/or state might be null if JvmtiDynamicCodeEventCollector
// has been initialized while JVMTI_EVENT_DYNAMIC_CODE_GENERATED was disabled.
- JvmtiThreadState *state = get_jvmti_thread_state(thread);
+ JvmtiThreadState *state = get_jvmti_thread_state(thread, false /* allow_suspend = false */ );
if (state != nullptr) {
JvmtiDynamicCodeEventCollector *collector = state->get_dynamic_code_event_collector();
if (collector != nullptr) {
diff --git a/src/hotspot/share/prims/jvmtiExport.hpp b/src/hotspot/share/prims/jvmtiExport.hpp
index 324e437411a..f4bede4d76e 100644
--- a/src/hotspot/share/prims/jvmtiExport.hpp
+++ b/src/hotspot/share/prims/jvmtiExport.hpp
@@ -309,7 +309,7 @@ class JvmtiExport : public AllStatic {
// If the jvmti_thread_state is absent and any thread filtered event
// is enabled globally then it is created.
// Otherwise, the thread->jvmti_thread_state() is returned.
- static JvmtiThreadState* get_jvmti_thread_state(JavaThread *thread);
+ static JvmtiThreadState* get_jvmti_thread_state(JavaThread *thread, bool allow_suspend = true);
// single stepping management methods
static void at_single_stepping_point(JavaThread *thread, Method* method, address location) NOT_JVMTI_RETURN;
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23490#discussion_r1969530676
More information about the hotspot-runtime-dev
mailing list