From 0ee34c280a7b030ff589788d90ffc8e6657ec152 Mon Sep 17 00:00:00 2001 From: zgu Date: Tue, 3 Sep 2019 06:57:35 +0100 Subject: [PATCH] 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: shade, andrew --- src/share/vm/ci/ciStreams.cpp | 9 ++- src/share/vm/ci/ciStreams.hpp | 4 +- src/share/vm/oops/instanceKlass.hpp | 5 ++ src/share/vm/prims/jvmtiRedefineClasses.cpp | 73 ++++++++++++++++----- src/share/vm/prims/jvmtiRedefineClasses.hpp | 4 ++ src/share/vm/runtime/mutexLocker.cpp | 2 + src/share/vm/runtime/mutexLocker.hpp | 1 + 7 files changed, 74 insertions(+), 24 deletions(-) diff --git a/src/share/vm/ci/ciStreams.cpp b/src/share/vm/ci/ciStreams.cpp index 76520fdff..a4eaf4739 100644 --- a/src/share/vm/ci/ciStreams.cpp +++ b/src/share/vm/ci/ciStreams.cpp @@ -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 @@ int ciBytecodeStream::get_method_index() { 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 @@ int ciBytecodeStream::get_method_holder_index() { // 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); diff --git a/src/share/vm/ci/ciStreams.hpp b/src/share/vm/ci/ciStreams.hpp index 091aa1bdf..07e573b59 100644 --- a/src/share/vm/ci/ciStreams.hpp +++ b/src/share/vm/ci/ciStreams.hpp @@ -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 @@ public: 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(); diff --git a/src/share/vm/oops/instanceKlass.hpp b/src/share/vm/oops/instanceKlass.hpp index a5f2eb346..444eadd38 100644 --- a/src/share/vm/oops/instanceKlass.hpp +++ b/src/share/vm/oops/instanceKlass.hpp @@ -225,6 +225,7 @@ class InstanceKlass: public Klass { // _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 bool _has_unloaded_dependent; enum { @@ -667,6 +668,10 @@ class InstanceKlass: public Klass { _nonstatic_oop_map_size = words; } + // 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); diff --git a/src/share/vm/prims/jvmtiRedefineClasses.cpp b/src/share/vm/prims/jvmtiRedefineClasses.cpp index 0e194b6f4..aac5f088d 100644 --- a/src/share/vm/prims/jvmtiRedefineClasses.cpp +++ b/src/share/vm/prims/jvmtiRedefineClasses.cpp @@ -67,6 +67,43 @@ VM_RedefineClasses::VM_RedefineClasses(jint class_count, _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; @@ -89,12 +126,21 @@ bool VM_RedefineClasses::doit_prologue() { _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 // better than adding a bunch of stop() calls. RC_TIMER_START(_timer_vm_op_prologue); + 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()); @@ -111,6 +157,7 @@ bool VM_RedefineClasses::doit_prologue() { // Free os::malloc allocated memory in load_new_class_version. os::free(_scratch_classes); RC_TIMER_STOP(_timer_vm_op_prologue); + unlock_classes(); return false; } @@ -170,6 +217,8 @@ void VM_RedefineClasses::doit() { } void VM_RedefineClasses::doit_epilogue() { + unlock_classes(); + // Free os::malloc allocated memory. os::free(_scratch_classes); @@ -961,14 +1010,7 @@ jvmtiError VM_RedefineClasses::load_new_class_versions(TRAPS) { // 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(); // RC_TRACE_WITH_THREAD macro has an embedded ResourceMark @@ -3855,22 +3897,19 @@ void VM_RedefineClasses::redefine_single_class(jclass the_jclass, HandleMark hm(THREAD); // make sure handles from this call are freed RC_TIMER_START(_timer_rsc_phase1); - 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(); @@ -4094,14 +4133,14 @@ void VM_RedefineClasses::redefine_single_class(jclass the_jclass, RC_TRACE_WITH_THREAD(0x00000001, THREAD, ("redefined name=%s, count=%d (avail_mem=" UINT64_FORMAT "K)", the_class->external_name(), - java_lang_Class::classRedefinedCount(the_class_mirror), + java_lang_Class::classRedefinedCount(the_class->java_mirror()), os::available_memory() >> 10)); { ResourceMark rm(THREAD); 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())); } RC_TIMER_STOP(_timer_rsc_phase2); diff --git a/src/share/vm/prims/jvmtiRedefineClasses.hpp b/src/share/vm/prims/jvmtiRedefineClasses.hpp index aab9d4549..167c01c31 100644 --- a/src/share/vm/prims/jvmtiRedefineClasses.hpp +++ b/src/share/vm/prims/jvmtiRedefineClasses.hpp @@ -490,6 +490,10 @@ class VM_RedefineClasses: public VM_Operation { 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 diff --git a/src/share/vm/runtime/mutexLocker.cpp b/src/share/vm/runtime/mutexLocker.cpp index f358c75ea..1f61967aa 100644 --- a/src/share/vm/runtime/mutexLocker.cpp +++ b/src/share/vm/runtime/mutexLocker.cpp @@ -125,6 +125,7 @@ Monitor* GCTaskManager_lock = NULL; Mutex* Management_lock = NULL; Monitor* Service_lock = NULL; Monitor* PeriodicTask_lock = NULL; +Monitor* RedefineClasses_lock = NULL; #ifdef INCLUDE_TRACE Mutex* JfrStacktrace_lock = NULL; @@ -279,6 +280,7 @@ void mutex_init() { def(ProfileVM_lock , Monitor, special, false); // used for profiling of the VMThread def(CompileThread_lock , Monitor, nonleaf+5, false ); def(PeriodicTask_lock , Monitor, nonleaf+5, true); + def(RedefineClasses_lock , Monitor, nonleaf+5, true); #ifdef INCLUDE_TRACE def(JfrMsg_lock , Monitor, leaf, true); diff --git a/src/share/vm/runtime/mutexLocker.hpp b/src/share/vm/runtime/mutexLocker.hpp index be86bac71..138e30e83 100644 --- a/src/share/vm/runtime/mutexLocker.hpp +++ b/src/share/vm/runtime/mutexLocker.hpp @@ -141,6 +141,7 @@ extern Mutex* MMUTracker_lock; // protects the MMU 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 -- GitLab