8155951: VM crash in nsk/jvmti/RedefineClasses/StressRedefine: assert failed: Corrupted constant pool
8151066: assert(0 <= i && i < length()) failed: index out of bounds
Summary: lock classes for redefinition because constant pool merging isn't thread safe, use method constant pool because constant pool merging doesn't make equivalent cpCaches because of invokedynamic
Reviewed-by: sspitsyn, dholmes
--- a/hotspot/src/share/vm/ci/ciStreams.cpp Tue May 17 03:26:07 2016 -0400
+++ b/hotspot/src/share/vm/ci/ciStreams.cpp Tue May 17 08:51:05 2016 -0400
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1999, 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1999, 2016, 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
@@ -361,14 +361,14 @@
ciMethod* ciBytecodeStream::get_method(bool& will_link, ciSignature* *declared_signature_result) {
VM_ENTRY_MARK;
ciEnv* env = CURRENT_ENV;
- constantPoolHandle cpool(_method->get_Method()->constants());
+ constantPoolHandle cpool(THREAD, _method->get_Method()->constants());
ciMethod* m = env->get_method_by_index(cpool, get_method_index(), cur_bc(), _holder);
will_link = m->is_loaded();
// Use the MethodType stored in the CP cache to create a signature
// with correct types (in respect to class loaders).
if (has_method_type()) {
- ciSymbol* sig_sym = env->get_symbol(cpool->symbol_at(get_method_signature_index()));
+ ciSymbol* sig_sym = env->get_symbol(cpool->symbol_at(get_method_signature_index(cpool)));
ciKlass* pool_holder = env->get_klass(cpool->pool_holder());
ciMethodType* method_type = get_method_type();
ciSignature* declared_signature = new (env->arena()) ciSignature(pool_holder, sig_sym, method_type);
@@ -465,9 +465,8 @@
// Get the constant pool index of the signature of the method
// referenced by the current bytecode. Used for generating
// deoptimization information.
-int ciBytecodeStream::get_method_signature_index() {
+int ciBytecodeStream::get_method_signature_index(const constantPoolHandle& cpool) {
GUARDED_VM_ENTRY(
- ConstantPool* cpool = _holder->get_instanceKlass()->constants();
const int method_index = get_method_index();
const int name_and_type_index = cpool->name_and_type_ref_index_at(method_index);
return cpool->signature_ref_index_at(name_and_type_index);
--- a/hotspot/src/share/vm/ci/ciStreams.hpp Tue May 17 03:26:07 2016 -0400
+++ b/hotspot/src/share/vm/ci/ciStreams.hpp Tue May 17 08:51:05 2016 -0400
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1999, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1999, 2016, 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
@@ -264,7 +264,7 @@
ciMethodType* get_method_type();
ciKlass* get_declared_method_holder();
int get_method_holder_index();
- int get_method_signature_index();
+ int get_method_signature_index(const constantPoolHandle& cpool);
// Get the resolved references arrays from the constant pool
ciObjArray* get_resolved_references();
--- a/hotspot/src/share/vm/oops/instanceKlass.hpp Tue May 17 03:26:07 2016 -0400
+++ b/hotspot/src/share/vm/oops/instanceKlass.hpp Tue May 17 08:51:05 2016 -0400
@@ -190,6 +190,7 @@
// _is_marked_dependent can be set concurrently, thus cannot be part of the
// _misc_flags.
bool _is_marked_dependent; // used for marking during flushing and deoptimization
+ bool _is_being_redefined; // used for locking redefinition
// The low two bits of _misc_flags contains the kind field.
// This can be used to quickly discriminate among the four kinds of
@@ -694,6 +695,10 @@
}
#if INCLUDE_JVMTI
+ // Redefinition locking. Class can only be redefined by one thread at a time.
+ bool is_being_redefined() const { return _is_being_redefined; }
+ void set_is_being_redefined(bool value) { _is_being_redefined = value; }
+
// RedefineClasses() support for previous versions:
void add_previous_version(instanceKlassHandle ikh, int emcp_method_count);
--- a/hotspot/src/share/vm/prims/jvmtiRedefineClasses.cpp Tue May 17 03:26:07 2016 -0400
+++ b/hotspot/src/share/vm/prims/jvmtiRedefineClasses.cpp Tue May 17 08:51:05 2016 -0400
@@ -69,6 +69,43 @@
_res = JVMTI_ERROR_NONE;
}
+static inline InstanceKlass* get_ik(jclass def) {
+ oop mirror = JNIHandles::resolve_non_null(def);
+ return InstanceKlass::cast(java_lang_Class::as_Klass(mirror));
+}
+
+// If any of the classes are being redefined, wait
+// Parallel constant pool merging leads to indeterminate constant pools.
+void VM_RedefineClasses::lock_classes() {
+ MutexLocker ml(RedefineClasses_lock);
+ bool has_redefined;
+ do {
+ has_redefined = false;
+ // Go through classes each time until none are being redefined.
+ for (int i = 0; i < _class_count; i++) {
+ if (get_ik(_class_defs[i].klass)->is_being_redefined()) {
+ RedefineClasses_lock->wait();
+ has_redefined = true;
+ break; // for loop
+ }
+ }
+ } while (has_redefined);
+ for (int i = 0; i < _class_count; i++) {
+ get_ik(_class_defs[i].klass)->set_is_being_redefined(true);
+ }
+ RedefineClasses_lock->notify_all();
+}
+
+void VM_RedefineClasses::unlock_classes() {
+ MutexLocker ml(RedefineClasses_lock);
+ for (int i = 0; i < _class_count; i++) {
+ assert(get_ik(_class_defs[i].klass)->is_being_redefined(),
+ "should be being redefined to get here");
+ get_ik(_class_defs[i].klass)->set_is_being_redefined(false);
+ }
+ RedefineClasses_lock->notify_all();
+}
+
bool VM_RedefineClasses::doit_prologue() {
if (_class_count == 0) {
_res = JVMTI_ERROR_NONE;
@@ -91,6 +128,14 @@
_res = JVMTI_ERROR_NULL_POINTER;
return false;
}
+
+ oop mirror = JNIHandles::resolve_non_null(_class_defs[i].klass);
+ // classes for primitives and arrays cannot be redefined
+ // check here so following code can assume these classes are InstanceKlass
+ if (!is_modifiable_class(mirror)) {
+ _res = JVMTI_ERROR_UNMODIFIABLE_CLASS;
+ return false;
+ }
}
// Start timer after all the sanity checks; not quite accurate, but
@@ -99,6 +144,7 @@
_timer_vm_op_prologue.start();
}
+ lock_classes();
// We first load new class versions in the prologue, because somewhere down the
// call chain it is required that the current thread is a Java thread.
_res = load_new_class_versions(Thread::current());
@@ -115,6 +161,7 @@
// Free os::malloc allocated memory in load_new_class_version.
os::free(_scratch_classes);
_timer_vm_op_prologue.stop();
+ unlock_classes();
return false;
}
@@ -174,6 +221,8 @@
}
void VM_RedefineClasses::doit_epilogue() {
+ unlock_classes();
+
// Free os::malloc allocated memory.
os::free(_scratch_classes);
@@ -959,14 +1008,7 @@
// versions are deleted. Constant pools are deallocated while merging
// constant pools
HandleMark hm(THREAD);
-
- oop mirror = JNIHandles::resolve_non_null(_class_defs[i].klass);
- // classes for primitives cannot be redefined
- if (!is_modifiable_class(mirror)) {
- return JVMTI_ERROR_UNMODIFIABLE_CLASS;
- }
- Klass* the_class_oop = java_lang_Class::as_Klass(mirror);
- instanceKlassHandle the_class = instanceKlassHandle(THREAD, the_class_oop);
+ instanceKlassHandle the_class(THREAD, get_ik(_class_defs[i].klass));
Symbol* the_class_sym = the_class->name();
log_debug(redefine, class, load)
@@ -3757,22 +3799,19 @@
_timer_rsc_phase1.start();
}
- instanceKlassHandle scratch_class(scratch_class_oop);
-
- oop the_class_mirror = JNIHandles::resolve_non_null(the_jclass);
- Klass* the_class_oop = java_lang_Class::as_Klass(the_class_mirror);
- instanceKlassHandle the_class = instanceKlassHandle(THREAD, the_class_oop);
+ instanceKlassHandle scratch_class(THREAD, scratch_class_oop);
+ instanceKlassHandle the_class(THREAD, get_ik(the_jclass));
// Remove all breakpoints in methods of this class
JvmtiBreakpoints& jvmti_breakpoints = JvmtiCurrentBreakpoints::get_jvmti_breakpoints();
- jvmti_breakpoints.clearall_in_class_at_safepoint(the_class_oop);
+ jvmti_breakpoints.clearall_in_class_at_safepoint(the_class());
// Deoptimize all compiled code that depends on this class
flush_dependent_code(the_class, THREAD);
_old_methods = the_class->methods();
_new_methods = scratch_class->methods();
- _the_class_oop = the_class_oop;
+ _the_class_oop = the_class();
compute_added_deleted_matching_methods();
update_jmethod_ids();
@@ -4002,10 +4041,10 @@
increment_class_counter((InstanceKlass *)the_class(), THREAD);
log_info(redefine, class, load)
("redefined name=%s, count=%d (avail_mem=" UINT64_FORMAT "K)",
- the_class->external_name(), java_lang_Class::classRedefinedCount(the_class_mirror), os::available_memory() >> 10);
+ the_class->external_name(), java_lang_Class::classRedefinedCount(the_class->java_mirror()), os::available_memory() >> 10);
Events::log_redefinition(THREAD, "redefined class name=%s, count=%d",
the_class->external_name(),
- java_lang_Class::classRedefinedCount(the_class_mirror));
+ java_lang_Class::classRedefinedCount(the_class->java_mirror()));
}
_timer_rsc_phase2.stop();
--- a/hotspot/src/share/vm/prims/jvmtiRedefineClasses.hpp Tue May 17 03:26:07 2016 -0400
+++ b/hotspot/src/share/vm/prims/jvmtiRedefineClasses.hpp Tue May 17 08:51:05 2016 -0400
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2003, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2016, 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
@@ -489,6 +489,10 @@
void flush_dependent_code(instanceKlassHandle k_h, TRAPS);
+ // lock classes to redefine since constant pool merging isn't thread safe.
+ void lock_classes();
+ void unlock_classes();
+
static void dump_methods();
// Check that there are no old or obsolete methods
--- a/hotspot/src/share/vm/runtime/mutexLocker.cpp Tue May 17 03:26:07 2016 -0400
+++ b/hotspot/src/share/vm/runtime/mutexLocker.cpp Tue May 17 08:51:05 2016 -0400
@@ -120,6 +120,7 @@
Mutex* Management_lock = NULL;
Monitor* Service_lock = NULL;
Monitor* PeriodicTask_lock = NULL;
+Monitor* RedefineClasses_lock = NULL;
#ifdef INCLUDE_TRACE
Mutex* JfrStacktrace_lock = NULL;
@@ -264,6 +265,7 @@
def(Debug3_lock , Mutex , nonleaf+4, true, Monitor::_safepoint_check_never);
def(CompileThread_lock , Monitor, nonleaf+5, false, Monitor::_safepoint_check_always);
def(PeriodicTask_lock , Monitor, nonleaf+5, true, Monitor::_safepoint_check_sometimes);
+ def(RedefineClasses_lock , Monitor, nonleaf+5, true, Monitor::_safepoint_check_always);
if (WhiteBoxAPI) {
def(Compilation_lock , Monitor, leaf, false, Monitor::_safepoint_check_never);
}
--- a/hotspot/src/share/vm/runtime/mutexLocker.hpp Tue May 17 03:26:07 2016 -0400
+++ b/hotspot/src/share/vm/runtime/mutexLocker.hpp Tue May 17 08:51:05 2016 -0400
@@ -121,6 +121,7 @@
extern Mutex* Management_lock; // a lock used to serialize JVM management
extern Monitor* Service_lock; // a lock used for service thread operation
extern Monitor* PeriodicTask_lock; // protects the periodic task structure
+extern Monitor* RedefineClasses_lock; // locks classes from parallel redefinition
#ifdef INCLUDE_TRACE
extern Mutex* JfrStacktrace_lock; // used to guard access to the JFR stacktrace table