7124710: interleaved RedefineClasses() and RetransformClasses() calls may have a problem

Reviewed-by: sspitsyn, dcubed
This commit is contained in:
Alex Menkov 2022-10-07 23:02:19 +00:00
parent d39d8c856a
commit 495c043533
4 changed files with 585 additions and 17 deletions

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2015, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2015, 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
@ -123,7 +123,10 @@ static ClassFileStream* check_class_file_load_hook(ClassFileStream* stream,
Handle class_loader(THREAD, loader_data->class_loader());
// Get the cached class file bytes (if any) from the class that
// is being redefined or retransformed. We use jvmti_thread_state()
// is being retransformed. If class file load hook provides
// modified class data during class loading or redefinition,
// new cached class file buffer should be allocated.
// We use jvmti_thread_state()
// instead of JvmtiThreadState::state_for(jt) so we don't allocate
// a JvmtiThreadState any earlier than necessary. This will help
// avoid the bug described by 7126851.
@ -132,8 +135,7 @@ static ClassFileStream* check_class_file_load_hook(ClassFileStream* stream,
if (state != NULL) {
Klass* k = state->get_class_being_redefined();
if (k != NULL) {
if (k != NULL && state->get_class_load_kind() == jvmti_class_load_kind_retransform) {
InstanceKlass* class_being_redefined = InstanceKlass::cast(k);
*cached_class_file = class_being_redefined->get_cached_class_file();
}

View File

@ -4329,20 +4329,18 @@ void VM_RedefineClasses::redefine_single_class(Thread* current, jclass the_jclas
int emcp_method_count = check_methods_and_mark_as_obsolete();
transfer_old_native_function_registrations(the_class);
// The class file bytes from before any retransformable agents mucked
// with them was cached on the scratch class, move to the_class.
// Note: we still want to do this if nothing needed caching since it
// should get cleared in the_class too.
if (the_class->get_cached_class_file() == 0) {
// the_class doesn't have a cache yet so copy it
the_class->set_cached_class_file(scratch_class->get_cached_class_file());
}
else if (scratch_class->get_cached_class_file() !=
the_class->get_cached_class_file()) {
// The same class can be present twice in the scratch classes list or there
if (scratch_class->get_cached_class_file() != the_class->get_cached_class_file()) {
// 1. the_class doesn't have a cache yet, scratch_class does have a cache.
// 2. The same class can be present twice in the scratch classes list or there
// are multiple concurrent RetransformClasses calls on different threads.
// In such cases we have to deallocate scratch_class cached_class_file.
os::free(scratch_class->get_cached_class_file());
// the_class and scratch_class have the same cached bytes, but different buffers.
// In such cases we need to deallocate one of the buffers.
// 3. RedefineClasses and the_class has cached bytes from a previous transformation.
// In the case we need to use class bytes from scratch_class.
if (the_class->get_cached_class_file() != nullptr) {
os::free(the_class->get_cached_class_file());
}
the_class->set_cached_class_file(scratch_class->get_cached_class_file());
}
// NULL out in scratch class to not delete twice. The class to be redefined

View File

@ -0,0 +1,274 @@
/*
* Copyright (c) 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.
*
* 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.
*/
/*
* @test
*
* @bug 7124710
*
* @requires vm.jvmti
* @modules java.base/jdk.internal.org.objectweb.asm
* @library /test/lib
*
* @comment main/othervm/native -Xlog:redefine*=trace -agentlib:RedefineRetransform RedefineRetransform
* @run main/othervm/native -agentlib:RedefineRetransform RedefineRetransform 1
* @run main/othervm/native -agentlib:RedefineRetransform RedefineRetransform 2
* @run main/othervm/native -agentlib:RedefineRetransform RedefineRetransform 3
* @run main/othervm/native -agentlib:RedefineRetransform RedefineRetransform 4
* @run main/othervm/native -agentlib:RedefineRetransform RedefineRetransform 5
* @run main/othervm/native -agentlib:RedefineRetransform RedefineRetransform 6
*/
import java.io.IOException;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import jdk.internal.org.objectweb.asm.AnnotationVisitor;
import jdk.internal.org.objectweb.asm.ClassReader;
import jdk.internal.org.objectweb.asm.ClassVisitor;
import jdk.internal.org.objectweb.asm.ClassWriter;
import jdk.internal.org.objectweb.asm.Opcodes;
import jdk.internal.org.objectweb.asm.Type;
/*
* The test verifies that after interleaved RedefineClasses/RetransformClasses calls
* JVMTI passes correct class bytes to ClassFileLoadHook (as per JVMTI spec).
* To distinguish class version the test instruments test class overriding runtime-visible annotation.
*/
public class RedefineRetransform {
static {
System.loadLibrary("RedefineRetransform");
}
@Retention(RetentionPolicy.RUNTIME)
@interface ClassVersion {
int value();
}
// Use runtime-visible annotation to specify class version.
@ClassVersion(0)
static class TestClass {
public TestClass() { }
}
// Redefines testClass with classBytes, instruments with classLoadHookBytes (if != null).
// Returns class bytes passed to ClassFileLoadHook or null on error.
private static native byte[] nRedefine(Class testClass, byte[] classBytes, byte[] classLoadHookBytes);
// Retransforms testClass, instruments with classBytes (if != null).
// Returns class bytes passed to ClassFileLoadHook or null on error.
private static native byte[] nRetransform(Class testClass, byte[] classBytes);
// Class bytes for initial TestClass (ClassVersion == 0).
private static byte[] initialClassBytes;
private static class VersionScanner extends ClassVisitor {
private Integer detectedVersion;
private Integer versionToSet;
// to get version
public VersionScanner() {
super(Opcodes.ASM7);
}
// to set version
public VersionScanner(int verToSet, ClassVisitor classVisitor) {
super(Opcodes.ASM7, classVisitor);
versionToSet = verToSet;
}
public int detectedVersion() {
if (detectedVersion == null) {
throw new RuntimeException("Version not detected");
}
return detectedVersion;
}
@Override
public AnnotationVisitor visitAnnotation(String descriptor, boolean visible) {
//log("visitAnnotation: descr = '" + descriptor + "', visible = " + visible);
if (Type.getDescriptor(ClassVersion.class).equals(descriptor)) {
return new AnnotationVisitor(Opcodes.ASM7, super.visitAnnotation(descriptor, visible)) {
@Override
public void visit(String name, Object value) {
//log("visit: name = '" + name + "', value = " + value
// + " (" + (value == null ? "N/A" : value.getClass()) + ")");
if ("value".equals(name) && value instanceof Integer intValue) {
detectedVersion = intValue;
if (versionToSet != null) {
//log("replace with " + versionToSet);
value = versionToSet;
}
}
super.visit(name, value);
}
};
}
return super.visitAnnotation(descriptor, visible);
}
}
// Generates TestClass class bytes with the specified ClassVersion value.
private static byte[] getClassBytes(int ver) {
if (ver < 0) {
return null;
}
ClassWriter cw = new ClassWriter(0);
ClassReader cr = new ClassReader(initialClassBytes);
cr.accept(new VersionScanner(ver, cw), 0);
return cw.toByteArray();
}
// Extracts ClassVersion values from the provided class bytes.
private static int getClassBytesVersion(byte[] classBytes) {
ClassReader cr = new ClassReader(classBytes);
VersionScanner scanner = new VersionScanner();
cr.accept(scanner, 0);
return scanner.detectedVersion();
}
static void init() {
try {
initialClassBytes = TestClass.class.getClassLoader()
.getResourceAsStream("RedefineRetransform$TestClass.class")
.readAllBytes();
log("Read TestClass bytes: " + initialClassBytes.length);
} catch (IOException ex) {
throw new RuntimeException("Failed to read class bytes", ex);
}
}
// Redefines TestClass to the version specified.
static void redefine(int ver) {
redefine(ver, -1);
}
// Redefines TestClass to the version specified
// instrumenting (from ClassFileLoadHook) with 'classLoadHookVer' class bytes (if >= 0).
// Also verifies that class bytes passed to ClassFileLoadHook have correct version (ver).
static void redefine(int ver, int classLoadHookVer) {
byte[] classBytes = getClassBytes(ver);
byte[] classLoadHookBytes = getClassBytes(classLoadHookVer);
byte[] hookClassBytes = nRedefine(TestClass.class, classBytes, classLoadHookBytes);
if (hookClassBytes == null) {
throw new RuntimeException("Redefine error (ver = " + ver + ")");
}
// verify ClassFileLoadHook gets the expected class bytes
int hookVer = getClassBytesVersion(hookClassBytes);
if (hookVer != ver) {
throw new RuntimeException("CLFH got unexpected version: " + hookVer
+ " (expected " + ver + ")");
}
}
// Retransforms TestClass instrumenting (from ClassFileLoadHook) with 'ver' class bytes (if >= 0).
// Verifies that class bytes passed to ClassFileLoadHook have correct version (expectedVer).
static void retransform(int ver, int expectedVer) {
byte[] classBytes = getClassBytes(ver);
byte[] hookClassBytes = nRetransform(TestClass.class, classBytes);
int hookVer = getClassBytesVersion(hookClassBytes);
if (hookVer != expectedVer) {
throw new RuntimeException("CLFH got unexpected version: " + hookVer
+ " (expected " + expectedVer + ")");
}
}
public static void main(String[] args) throws Exception {
int testCase;
try {
testCase = Integer.valueOf(args[0]);
} catch (Exception ex) {
throw new RuntimeException("Single numeric argument expected", ex);
}
init();
switch (testCase) {
case 1:
test("Redefine-Retransform-Retransform", () -> {
redefine(1); // cached class bytes are not set
retransform(2, 1); // sets cached class bytes to ver 1
retransform(3, 1); // uses existing cache
});
break;
case 2:
test("Redefine-Retransform-Redefine-Redefine", () -> {
redefine(1); // cached class bytes are not set
retransform(2, 1); // sets cached class bytes to ver 1
redefine(3); // resets cached class bytes to nullptr
redefine(4); // cached class bytes are not set
});
break;
case 3:
test("Redefine-Retransform-Redefine-Retransform", () -> {
redefine(1); // cached class bytes are not set
retransform(2, 1); // sets cached class bytes to ver 1
redefine(3); // resets cached class bytes to nullptr
retransform(4, 3); // sets cached class bytes to ver 3
});
break;
case 4:
test("Retransform-Redefine-Retransform-Retransform", () -> {
retransform(1, 0); // sets cached class bytes to ver 0 (initially loaded)
redefine(2); // resets cached class bytes to nullptr
retransform(3, 2); // sets cached class bytes to ver 2
retransform(4, 2); // uses existing cache
});
break;
case 5:
test("Redefine-Retransform-Redefine-Retransform with CFLH", () -> {
redefine(1, 5); // CFLH sets cached class bytes to ver 1
retransform(2, 1); // uses existing cache
redefine(3, 6); // resets cached class bytes to nullptr,
// CFLH sets cached class bytes to ver 3
retransform(4, 3); // uses existing cache
});
break;
case 6:
test("Retransform-Redefine-Retransform-Retransform with CFLH", () -> {
retransform(1, 0); // sets cached class bytes to ver 0 (initially loaded)
redefine(2, 5); // resets cached class bytes to nullptr,
// CFLH sets cached class bytes to ver 2
retransform(3, 2); // uses existing cache
retransform(4, 2); // uses existing cache
});
break;
}
}
private static void log(Object msg) {
System.out.println(msg);
}
private interface Test {
void test();
}
private static void test(String name, Test theTest) {
log(">>Test: " + name);
theTest.test();
log("<<Test: " + name + " - OK");
log("");
}
}

View File

@ -0,0 +1,294 @@
/*
* Copyright (c) 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.
*
* 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.
*/
#include <jni.h>
#include <jvmti.h>
#include <stdio.h>
#include <string.h>
// set by Agent_OnLoad
static jvmtiEnv* jvmti = nullptr;
static const char testClassName[] = "RedefineRetransform$TestClass";
static void _log(const char* format, ...) {
va_list args;
va_start(args, format);
vprintf(format, args);
va_end(args);
fflush(0);
}
static bool isTestClass(const char* name) {
return name != nullptr && strcmp(name, testClassName) == 0;
}
/*
* Helper class for data exchange between RedefineClasses/RetransformClasses and
* ClassFileLoadHook callback (saves class bytes to be passed to CFLH,
* allows setting new class bytes to return from CFLH).
* Callers create an instance on the stack, ClassFileLoadHook handler uses getInstance().
*/
class ClassFileLoadHookHelper {
const char* mode; // for logging only
bool eventEnabled;
JNIEnv* env;
jbyteArray classBytes = nullptr;
unsigned char* savedClassBytes = nullptr;
jint savedClassBytesLen = 0;
// single instance
static ClassFileLoadHookHelper *instance;
public:
ClassFileLoadHookHelper(const char* mode, JNIEnv* jni_env, jbyteArray hookClassBytes)
: mode(mode), eventEnabled(false), env(jni_env), classBytes(nullptr),
savedClassBytes(nullptr), savedClassBytesLen(0)
{
_log(">>%s\n", mode);
if (hookClassBytes != nullptr) {
classBytes = (jbyteArray)env->NewGlobalRef(hookClassBytes);
}
}
~ClassFileLoadHookHelper() {
// cleanup on error
stop();
if (classBytes != nullptr) {
env->DeleteGlobalRef(classBytes);
}
if (savedClassBytes != nullptr) {
jvmti->Deallocate(savedClassBytes);
}
_log("<<%s\n", mode);
}
bool start() {
instance = this;
jvmtiError err = jvmti->SetEventNotificationMode(JVMTI_ENABLE, JVMTI_EVENT_CLASS_FILE_LOAD_HOOK, nullptr);
if (err != JVMTI_ERROR_NONE) {
_log("%s: SetEventNotificationMode(JVMTI_ENABLE) error %d\n", mode, err);
eventEnabled = true;
return false;
}
return true;
}
void stop() {
instance = nullptr;
if (eventEnabled) {
jvmtiError err = jvmti->SetEventNotificationMode(JVMTI_DISABLE, JVMTI_EVENT_CLASS_FILE_LOAD_HOOK, nullptr);
if (err != JVMTI_ERROR_NONE) {
_log("%s: SetEventNotificationMode(JVMTI_DISABLE) error %d\n", mode, err);
return;
}
eventEnabled = false;
}
}
// valid only between start() and stop()
static ClassFileLoadHookHelper* getInstance() {
return instance;
}
bool getHookClassBytes(unsigned char** newClassBytes, jint* newLen) {
if (classBytes != nullptr) {
jsize len = env->GetArrayLength(classBytes);
unsigned char* buf = nullptr;
jvmtiError err = jvmti->Allocate(len, &buf);
if (err != JVMTI_ERROR_NONE) {
_log("ClassFileLoadHook: failed to allocate %ld bytes for new class bytes: %d", len, err);
return false;
}
jbyte* arrayPtr = env->GetByteArrayElements(classBytes, nullptr);
if (arrayPtr == nullptr) {
_log("ClassFileLoadHook: failed to get array elements\n");
jvmti->Deallocate(buf);
return false;
}
memcpy(buf, arrayPtr, len);
env->ReleaseByteArrayElements(classBytes, arrayPtr, JNI_ABORT);
*newClassBytes = buf;
*newLen = len;
_log(" ClassFileLoadHook: set new class bytes\n");
}
return true;
}
void setSavedHookClassBytes(const unsigned char* bytes, jint len) {
jvmtiError err = jvmti->Allocate(len, &savedClassBytes);
if (err != JVMTI_ERROR_NONE) {
_log("ClassFileLoadHook: failed to allocate %ld bytes for saved class bytes: %d", len, err);
return;
}
memcpy(savedClassBytes, bytes, len);
savedClassBytesLen = len;
}
jbyteArray getSavedHookClassBytes() {
if (savedClassBytes == nullptr) {
_log("%s: savedClassBytes is NULL\n", mode);
return nullptr;
}
jbyteArray result = env->NewByteArray(savedClassBytesLen);
if (result == nullptr) {
_log("%s: NewByteArray(%ld) failed\n", mode, savedClassBytesLen);
} else {
jbyte* arrayPtr = env->GetByteArrayElements(result, nullptr);
if (arrayPtr == nullptr) {
_log("%s: Failed to get array elements\n", mode);
result = nullptr;
} else {
memcpy(arrayPtr, savedClassBytes, savedClassBytesLen);
env->ReleaseByteArrayElements(result, arrayPtr, JNI_COMMIT);
}
}
return result;
}
};
ClassFileLoadHookHelper* ClassFileLoadHookHelper::instance = nullptr;
extern "C" {
JNIEXPORT void JNICALL
callbackClassFileLoadHook(jvmtiEnv *jvmti_env,
JNIEnv* jni_env,
jclass class_being_redefined,
jobject loader,
const char* name,
jobject protection_domain,
jint class_data_len,
const unsigned char* class_data,
jint* new_class_data_len,
unsigned char** new_class_data) {
if (isTestClass(name)) {
_log(">>ClassFileLoadHook: %s, %ld bytes, ptr = %p\n", name, class_data_len, class_data);
ClassFileLoadHookHelper* helper = ClassFileLoadHookHelper::getInstance();
if (helper == nullptr) {
_log("ClassFileLoadHook ERROR: helper instance is not initialized\n");
return;
}
// save class bytes
helper->setSavedHookClassBytes(class_data, class_data_len);
// set new class bytes
helper->getHookClassBytes(new_class_data, new_class_data_len);
_log("<<ClassFileLoadHook\n");
}
}
JNIEXPORT jint JNICALL Agent_OnLoad(JavaVM* jvm, char* options, void* reserved) {
jint res = jvm->GetEnv((void **)&jvmti, JVMTI_VERSION_1_1);
if (res != JNI_OK) {
_log("Failed to get JVMTI interface: %ld\n", res);
return JNI_ERR;
}
jvmtiCapabilities caps;
memset(&caps, 0, sizeof(caps));
caps.can_redefine_classes = 1;
caps.can_retransform_classes = 1;
jvmtiError err = jvmti->AddCapabilities(&caps);
if (err != JVMTI_ERROR_NONE) {
_log("Failed to add capabilities: %d\n", err);
return JNI_ERR;
}
jvmtiEventCallbacks eventCallbacks;
memset(&eventCallbacks, 0, sizeof(eventCallbacks));
eventCallbacks.ClassFileLoadHook = callbackClassFileLoadHook;
err = jvmti->SetEventCallbacks(&eventCallbacks, sizeof(eventCallbacks));
if (err != JVMTI_ERROR_NONE) {
_log("Error setting event callbacks: %d\n", err);
return JNI_ERR;
}
return JNI_OK;
}
JNIEXPORT void JNICALL
Agent_OnUnload(JavaVM* jvm) {
return;
}
JNIEXPORT jbyteArray JNICALL
Java_RedefineRetransform_nRedefine(JNIEnv* env, jclass klass,
jclass testClass, jbyteArray classBytes, jbyteArray classLoadHookBytes) {
ClassFileLoadHookHelper helper("nRedefine", env, classLoadHookBytes);
jsize len = env->GetArrayLength(classBytes);
jbyte* arrayPtr = env->GetByteArrayElements(classBytes, nullptr);
if (arrayPtr == nullptr) {
_log("nRedefine: Failed to get array elements\n");
return nullptr;
}
if (helper.start()) {
jvmtiClassDefinition classDef;
memset(&classDef, 0, sizeof(classDef));
classDef.klass = testClass;
classDef.class_byte_count = len;
classDef.class_bytes = (unsigned char*)arrayPtr;
jvmtiError err = jvmti->RedefineClasses(1, &classDef);
if (err != JVMTI_ERROR_NONE) {
_log("nRedefine: RedefineClasses error %d", err);
// don't exit here, need to cleanup
}
helper.stop();
}
env->ReleaseByteArrayElements(classBytes, arrayPtr, JNI_ABORT);
return helper.getSavedHookClassBytes();
}
JNIEXPORT jbyteArray JNICALL
Java_RedefineRetransform_nRetransform(JNIEnv* env, jclass klass, jclass testClass, jbyteArray classBytes) {
ClassFileLoadHookHelper helper("nRetransform", env, classBytes);
if (helper.start()) {
jvmtiError err = jvmti->RetransformClasses(1, &testClass);
if (err != JVMTI_ERROR_NONE) {
_log("nRetransform: RetransformClasses error %d\n", err);
// don't exit here, disable CFLH event
}
helper.stop();
}
return helper.getSavedHookClassBytes();
}
}