8155951: VM crash in nsk/jvmti/RedefineClasses/StressRedefine: assert failed:...

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

 /*
- * 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);

src/share/vm/ci/ciStreams.hpp

 /*
- * 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();

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);
 

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);

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

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);

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