From fae68ff0168de308908e9d6b94c58b48b91659d7 Mon Sep 17 00:00:00 2001
From: Coleen Phillimore <coleenp@openjdk.org>
Date: Thu, 19 Nov 2020 22:39:07 +0000
Subject: [PATCH] 8256640: assert(!m->is_old() || ik()->is_being_redefined())
 failed: old methods should not be in vtable

Reviewed-by: lfoltan, dcubed, dholmes
---
 src/hotspot/share/oops/instanceKlass.cpp |  6 ++++++
 src/hotspot/share/oops/klassVtable.cpp   | 23 ++++++++++++++++++-----
 2 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/src/hotspot/share/oops/instanceKlass.cpp b/src/hotspot/share/oops/instanceKlass.cpp
index 4c720bb9872e9..4f0edbfde90aa 100644
--- a/src/hotspot/share/oops/instanceKlass.cpp
+++ b/src/hotspot/share/oops/instanceKlass.cpp
@@ -2571,15 +2571,21 @@ void InstanceKlass::restore_unshareable_info(ClassLoaderData* loader_data, Handl
   for (int index = 0; index < num_methods; ++index) {
     methods->at(index)->restore_unshareable_info(CHECK);
   }
+#if INCLUDE_JVMTI
   if (JvmtiExport::has_redefined_a_class()) {
     // Reinitialize vtable because RedefineClasses may have changed some
     // entries in this vtable for super classes so the CDS vtable might
     // point to old or obsolete entries.  RedefineClasses doesn't fix up
     // vtables in the shared system dictionary, only the main one.
     // It also redefines the itable too so fix that too.
+    // First fix any default methods that point to a super class that may
+    // have been redefined.
+    bool trace_name_printed = false;
+    adjust_default_methods(&trace_name_printed);
     vtable().initialize_vtable(false, CHECK);
     itable().initialize_itable(false, CHECK);
   }
+#endif
 
   // restore constant pool resolved references
   constants()->restore_unshareable_info(CHECK);
diff --git a/src/hotspot/share/oops/klassVtable.cpp b/src/hotspot/share/oops/klassVtable.cpp
index 1b166d2734fb2..84e4592b6b9e4 100644
--- a/src/hotspot/share/oops/klassVtable.cpp
+++ b/src/hotspot/share/oops/klassVtable.cpp
@@ -222,13 +222,22 @@ void klassVtable::initialize_vtable(bool checkconstraints, TRAPS) {
           assert(def_vtable_indices->length() == len, "reinit vtable len?");
         }
         for (int i = 0; i < len; i++) {
-          methodHandle mh(THREAD, default_methods->at(i));
-          assert(!mh->is_private(), "private interface method in the default method list");
-          bool needs_new_entry = update_inherited_vtable(mh, super_vtable_len, i, checkconstraints, CHECK);
+          bool needs_new_entry;
+          {
+            // Reduce the scope of this handle so that it is fetched again.
+            // The methodHandle keeps it from being deleted by RedefineClasses while
+            // we're using it.
+            methodHandle mh(THREAD, default_methods->at(i));
+            assert(!mh->is_private(), "private interface method in the default method list");
+            needs_new_entry = update_inherited_vtable(mh, super_vtable_len, i, checkconstraints, CHECK);
+          }
 
           // needs new entry
           if (needs_new_entry) {
-            put_method_at(mh(), initialized);
+            // Refetch this default method in case of redefinition that might
+            // happen during constraint checking in the update_inherited_vtable call above.
+            Method* method = default_methods->at(i);
+            put_method_at(method, initialized);
             if (is_preinitialized_vtable()) {
               // At runtime initialize_vtable is rerun for a shared class
               // (loaded by the non-boot loader) as part of link_class_impl().
@@ -494,6 +503,11 @@ bool klassVtable::update_inherited_vtable(const methodHandle& target_method,
           allocate_new = false;
         }
 
+        // Set the vtable index before the constraint check safepoint, which potentially
+        // redefines this method if this method is a default method belonging to a
+        // super class or interface.
+        put_method_at(target_method(), i);
+
         // Do not check loader constraints for overpass methods because overpass
         // methods are created by the jvm to throw exceptions.
         if (checkconstraints && !target_method->is_overpass()) {
@@ -531,7 +545,6 @@ bool klassVtable::update_inherited_vtable(const methodHandle& target_method,
           }
         }
 
-        put_method_at(target_method(), i);
         overrides = true;
         if (!is_default) {
           target_method->set_vtable_index(i);