8165268: [BACKOUT] InstanceKlass::_previous_version_count goes negative

Reviewed-by: dcubed

hotspot/src/share/vm/classfile/classLoaderData.cpp

@@ -962,7 +962,7 @@ bool ClassLoaderDataGraph::do_unloading(BoolObjectClosure* is_alive_closure,
 
   // Mark metadata seen on the stack only so we can delete unneeded entries.
   // Only walk all metadata, including the expensive code cache walk, for Full GC
-  // and only if class redefinition occurred and if there are previous versions of
+  // and only if class redefinition and if there's previous versions of
   // Klasses to delete.
   bool walk_all_metadata = clean_previous_versions &&
                            JvmtiExport::has_redefined_a_class() &&

hotspot/src/share/vm/oops/instanceKlass.cpp

@@ -3370,7 +3370,6 @@ int InstanceKlass::_previous_version_count = 0;
 
 // Purge previous versions before adding new previous versions of the class.
 void InstanceKlass::purge_previous_versions(InstanceKlass* ik) {
-  assert(SafepointSynchronize::is_at_safepoint(), "only called at safepoint");
   if (ik->previous_versions() != NULL) {
     // This klass has previous versions so see what we can cleanup
     // while it is safe to do so.
@@ -3399,12 +3398,7 @@ void InstanceKlass::purge_previous_versions(InstanceKlass* ik) {
         // are executing.  Unlink this previous_version.
         // The previous version InstanceKlass is on the ClassLoaderData deallocate list
         // so will be deallocated during the next phase of class unloading.
-        //
+        log_trace(redefine, class, iklass, purge)("previous version " INTPTR_FORMAT " is dead", p2i(pv_node));
-        // Update count for class unloading.
-        _previous_version_count--;
-        log_trace(redefine, class, iklass, purge)
-          ("previous version " INTPTR_FORMAT " is dead.  previous_version_count = %d",
-           p2i(pv_node), _previous_version_count);
         // For debugging purposes.
         pv_node->set_is_scratch_class();
         pv_node->class_loader_data()->add_to_deallocate_list(pv_node);
@@ -3519,7 +3513,6 @@ void InstanceKlass::add_previous_version(instanceKlassHandle scratch_class,
                                          int emcp_method_count) {
   assert(Thread::current()->is_VM_thread(),
          "only VMThread can add previous versions");
-  assert(SafepointSynchronize::is_at_safepoint(), "only called at safepoint");
 
   ResourceMark rm;
   log_trace(redefine, class, iklass, add)
@@ -3543,6 +3536,8 @@ void InstanceKlass::add_previous_version(instanceKlassHandle scratch_class,
     // For debugging purposes.
     scratch_class->set_is_scratch_class();
     scratch_class->class_loader_data()->add_to_deallocate_list(scratch_class());
+    // Update count for class unloading.
+    _previous_version_count--;
     return;
   }
 
@@ -3570,14 +3565,12 @@ void InstanceKlass::add_previous_version(instanceKlassHandle scratch_class,
   }
 
   // Add previous version if any methods are still running.
-  // Update count for class unloading.
+  log_trace(redefine, class, iklass, add)("scratch class added; one of its methods is on_stack");
-  _previous_version_count++;
-  log_trace(redefine, class, iklass, add)
-    ("scratch class added; one of its methods is on_stack.  previous_version_count = %d",
-      _previous_version_count);
   assert(scratch_class->previous_versions() == NULL, "shouldn't have a previous version");
   scratch_class->link_previous_versions(previous_versions());
   link_previous_versions(scratch_class());
+  // Update count for class unloading.
+  _previous_version_count++;
 } // end add_previous_version()
 
 #endif // INCLUDE_JVMTI

hotspot/src/share/vm/oops/instanceKlass.hpp

@@ -771,10 +771,7 @@ public:
   static int  _previous_version_count;
  public:
   static void purge_previous_versions(InstanceKlass* ik);
-  static bool has_previous_versions() {
+  static bool has_previous_versions() { return _previous_version_count > 0; }
-    assert(_previous_version_count >= 0, "count should never be negative");
-    return _previous_version_count > 0;
-  }
 
   // JVMTI: Support for caching a class file before it is modified by an agent that can do retransformation
   void set_cached_class_file(JvmtiCachedClassFileData *data) {

hotspot/test/runtime/RedefineTests/RedefineCount.java

@@ -1,56 +0,0 @@
-/*
- * Copyright (c) 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
- * under the terms of the GNU General Public License version 2 only, as
- * published by the Free Software Foundation.
- *
- * This code is distributed in the hope that it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
- * version 2 for more details (a copy is included in the LICENSE file that
- * accompanied this code).
- *
- * You should have received a copy of the GNU General Public License version
- * 2 along with this work; if not, write to the Free Software Foundation,
- * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
- *
- * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
- * or visit www.oracle.com if you need additional information or have any
- * questions.
- */
-
-/*
- * @test
- * @bug 8164692
- * @summary Redefine previous_versions count goes negative
- * @library /test/lib
- * @modules java.base/jdk.internal.misc
- * @modules java.compiler
- *          java.instrument
- *          jdk.jartool/sun.tools.jar
- * @run main RedefineClassHelper
- * @run main/othervm -javaagent:redefineagent.jar -Xlog:redefine+class+iklass+add=trace,redefine+class+iklass+purge=trace RedefineCount
- */
-public class RedefineCount {
-
-    public static String newB =
-                "class RedefineCount$B {" +
-                "}";
-
-    static class B { }
-
-    public static void main(String[] args) throws Exception {
-
-        // Redefine a class and create some garbage
-        // Since there are no methods running, the previous version is never added to the
-        // previous_version_list and the count should stay zero and not go negative
-        RedefineClassHelper.redefineClass(B.class, newB);
-
-        for (int i = 0; i < 20 ; i++) {
-            String s = new String("some garbage");
-            System.gc();
-        }
-    }
-}