8155951: VM crash in nsk/jvmti/RedefineClasses/StressRedefine: assert failed: Corrupted constant pool
authorcoleenp
Tue, 17 May 2016 08:51:05 -0400
changeset 38309 9b8e9c373740
parent 38308 ee489b336cd9
child 38310 7cd2ec72450e
child 38311 9f2555351233
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
hotspot/src/share/vm/ci/ciStreams.cpp
hotspot/src/share/vm/ci/ciStreams.hpp
hotspot/src/share/vm/oops/instanceKlass.hpp
hotspot/src/share/vm/prims/jvmtiRedefineClasses.cpp
hotspot/src/share/vm/prims/jvmtiRedefineClasses.hpp
hotspot/src/share/vm/runtime/mutexLocker.cpp
hotspot/src/share/vm/runtime/mutexLocker.hpp
--- 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