From 391fb4399ad77226b11fc319534ae90c2aeebac3 Mon Sep 17 00:00:00 2001 From: Matt Date: Tue, 7 Jan 2025 14:53:52 -0500 Subject: [PATCH 1/2] Add safeguard for J9-specific jmethodid unloading behaviour (version-dependent) --- ddprof-lib/src/main/cpp/flightRecorder.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddprof-lib/src/main/cpp/flightRecorder.cpp b/ddprof-lib/src/main/cpp/flightRecorder.cpp index a83bd313c..8a37c2f52 100644 --- a/ddprof-lib/src/main/cpp/flightRecorder.cpp +++ b/ddprof-lib/src/main/cpp/flightRecorder.cpp @@ -150,7 +150,7 @@ void Lookup::fillJavaMethodInfo(MethodInfo *mi, jmethodID method, bool entry = false; if (VMMethod::check_jmethodID(method) && jvmti->GetMethodDeclaringClass(method, &method_class) == 0 && - jvmti->GetClassSignature(method_class, &class_name, NULL) == 0 && + ((!VM::isOpenJ9() || method_class != reinterpret_cast(-1)) && jvmti->GetClassSignature(method_class, &class_name, NULL) == 0) && jvmti->GetMethodName(method, &method_name, &method_sig, NULL) == 0) { if (first_time) { From 91258600165f1737df1794036e81b6225e5f2fb4 Mon Sep 17 00:00:00 2001 From: Matt Date: Wed, 8 Jan 2025 09:05:01 -0500 Subject: [PATCH 2/2] Added comments providing context on J9 crash safeguards --- ddprof-lib/src/main/cpp/flightRecorder.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/ddprof-lib/src/main/cpp/flightRecorder.cpp b/ddprof-lib/src/main/cpp/flightRecorder.cpp index 8a37c2f52..54cb1ee14 100644 --- a/ddprof-lib/src/main/cpp/flightRecorder.cpp +++ b/ddprof-lib/src/main/cpp/flightRecorder.cpp @@ -150,6 +150,13 @@ void Lookup::fillJavaMethodInfo(MethodInfo *mi, jmethodID method, bool entry = false; if (VMMethod::check_jmethodID(method) && jvmti->GetMethodDeclaringClass(method, &method_class) == 0 && + // On some older versions of J9, the JVMTI call to GetMethodDeclaringClass will return OK = 0, but when a + // classloader is unloaded they free all JNIIDs. This means that anyone holding on to a jmethodID is + // pointing to corrupt data and the behaviour is undefined. + // The behaviour is adjusted so that when asgct() is used or if `-XX:+KeepJNIIDs` is specified, + // when a classloader is unloaded, the jmethodIDs are not freed, but instead marked as -1. + // The nested check below is to mitigate these crashes. + // In more recent versions, the condition above will short-circuit safely. ((!VM::isOpenJ9() || method_class != reinterpret_cast(-1)) && jvmti->GetClassSignature(method_class, &class_name, NULL) == 0) && jvmti->GetMethodName(method, &method_name, &method_sig, NULL) == 0) {