# HG changeset patch # User coleenp # Date 1473276321 14400 # Node ID 59f3c8a69541066dcabdc3db7e401c2630907ea9 # Parent 04b19236aa980d57dc2a98cb1164c31122bea0f5 8165246: [REDO] InstanceKlass::_previous_version_count goes negative Summary: make _has_previous_version a boolean that is set to true when previous version of a class is added or during class unloading call to purge_previous_versions Reviewed-by: gtriantafill, dcubed, sspitsyn diff -r 04b19236aa98 -r 59f3c8a69541 hotspot/src/share/vm/classfile/classLoaderData.cpp --- a/hotspot/src/share/vm/classfile/classLoaderData.cpp Wed Sep 07 15:21:45 2016 +0200 +++ b/hotspot/src/share/vm/classfile/classLoaderData.cpp Wed Sep 07 15:25:21 2016 -0400 @@ -966,7 +966,7 @@ // Klasses to delete. bool walk_all_metadata = clean_previous_versions && JvmtiExport::has_redefined_a_class() && - InstanceKlass::has_previous_versions(); + InstanceKlass::has_previous_versions_and_reset(); MetadataOnStackMark md_on_stack(walk_all_metadata); // Save previous _unloading pointer for CMS which may add to unloading list before diff -r 04b19236aa98 -r 59f3c8a69541 hotspot/src/share/vm/oops/instanceKlass.cpp --- a/hotspot/src/share/vm/oops/instanceKlass.cpp Wed Sep 07 15:21:45 2016 +0200 +++ b/hotspot/src/share/vm/oops/instanceKlass.cpp Wed Sep 07 15:25:21 2016 -0400 @@ -3365,88 +3365,119 @@ #if INCLUDE_JVMTI -// RedefineClasses() support for previous versions: -int InstanceKlass::_previous_version_count = 0; - -// Purge previous versions before adding new previous versions of the class. -void InstanceKlass::purge_previous_versions(InstanceKlass* ik) { - if (ik->previous_versions() != NULL) { - // This klass has previous versions so see what we can cleanup - // while it is safe to do so. - - int deleted_count = 0; // leave debugging breadcrumbs - int live_count = 0; - ClassLoaderData* loader_data = ik->class_loader_data(); - assert(loader_data != NULL, "should never be null"); - - ResourceMark rm; - log_trace(redefine, class, iklass, purge)("%s: previous versions", ik->external_name()); - - // previous versions are linked together through the InstanceKlass - InstanceKlass* pv_node = ik->previous_versions(); - InstanceKlass* last = ik; - int version = 0; - - // check the previous versions list - for (; pv_node != NULL; ) { - - ConstantPool* pvcp = pv_node->constants(); - assert(pvcp != NULL, "cp ref was unexpectedly cleared"); - - if (!pvcp->on_stack()) { - // If the constant pool isn't on stack, none of the methods - // 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)); - // For debugging purposes. - pv_node->set_is_scratch_class(); - pv_node->class_loader_data()->add_to_deallocate_list(pv_node); - pv_node = pv_node->previous_versions(); - last->link_previous_versions(pv_node); - deleted_count++; - version++; - continue; - } else { - log_trace(redefine, class, iklass, purge)("previous version " INTPTR_FORMAT " is alive", p2i(pv_node)); - assert(pvcp->pool_holder() != NULL, "Constant pool with no holder"); - guarantee (!loader_data->is_unloading(), "unloaded classes can't be on the stack"); - live_count++; - } - - // At least one method is live in this previous version. - // Reset dead EMCP methods not to get breakpoints. - // All methods are deallocated when all of the methods for this class are no - // longer running. - Array* method_refs = pv_node->methods(); - if (method_refs != NULL) { - log_trace(redefine, class, iklass, purge)("previous methods length=%d", method_refs->length()); - for (int j = 0; j < method_refs->length(); j++) { - Method* method = method_refs->at(j); - - if (!method->on_stack()) { - // no breakpoints for non-running methods - if (method->is_running_emcp()) { - method->set_running_emcp(false); - } - } else { - assert (method->is_obsolete() || method->is_running_emcp(), - "emcp method cannot run after emcp bit is cleared"); - log_trace(redefine, class, iklass, purge) - ("purge: %s(%s): prev method @%d in version @%d is alive", - method->name()->as_C_string(), method->signature()->as_C_string(), j, version); +// RedefineClasses() support for previous versions + +// Globally, there is at least one previous version of a class to walk +// during class unloading, which is saved because old methods in the class +// are still running. Otherwise the previous version list is cleaned up. +bool InstanceKlass::_has_previous_versions = false; + +// Returns true if there are previous versions of a class for class +// unloading only. Also resets the flag to false. purge_previous_version +// will set the flag to true if there are any left, i.e., if there's any +// work to do for next time. This is to avoid the expensive code cache +// walk in CLDG::do_unloading(). +bool InstanceKlass::has_previous_versions_and_reset() { + bool ret = _has_previous_versions; + log_trace(redefine, class, iklass, purge)("Class unloading: has_previous_versions = %s", + ret ? "true" : "false"); + _has_previous_versions = false; + return ret; +} + +// Purge previous versions before adding new previous versions of the class and +// during class unloading. +void InstanceKlass::purge_previous_version_list() { + assert(SafepointSynchronize::is_at_safepoint(), "only called at safepoint"); + assert(has_been_redefined(), "Should only be called for main class"); + + // Quick exit. + if (previous_versions() == NULL) { + return; + } + + // This klass has previous versions so see what we can cleanup + // while it is safe to do so. + + int deleted_count = 0; // leave debugging breadcrumbs + int live_count = 0; + ClassLoaderData* loader_data = class_loader_data(); + assert(loader_data != NULL, "should never be null"); + + ResourceMark rm; + log_trace(redefine, class, iklass, purge)("%s: previous versions", external_name()); + + // previous versions are linked together through the InstanceKlass + InstanceKlass* pv_node = previous_versions(); + InstanceKlass* last = this; + int version = 0; + + // check the previous versions list + for (; pv_node != NULL; ) { + + ConstantPool* pvcp = pv_node->constants(); + assert(pvcp != NULL, "cp ref was unexpectedly cleared"); + + if (!pvcp->on_stack()) { + // If the constant pool isn't on stack, none of the methods + // 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)); + // For debugging purposes. + pv_node->set_is_scratch_class(); + // Unlink from previous version list. + assert(pv_node->class_loader_data() == loader_data, "wrong loader_data"); + InstanceKlass* next = pv_node->previous_versions(); + pv_node->link_previous_versions(NULL); // point next to NULL + last->link_previous_versions(next); + // Add to the deallocate list after unlinking + loader_data->add_to_deallocate_list(pv_node); + pv_node = next; + deleted_count++; + version++; + continue; + } else { + log_trace(redefine, class, iklass, purge)("previous version " INTPTR_FORMAT " is alive", p2i(pv_node)); + assert(pvcp->pool_holder() != NULL, "Constant pool with no holder"); + guarantee (!loader_data->is_unloading(), "unloaded classes can't be on the stack"); + live_count++; + // found a previous version for next time we do class unloading + _has_previous_versions = true; + } + + // At least one method is live in this previous version. + // Reset dead EMCP methods not to get breakpoints. + // All methods are deallocated when all of the methods for this class are no + // longer running. + Array* method_refs = pv_node->methods(); + if (method_refs != NULL) { + log_trace(redefine, class, iklass, purge)("previous methods length=%d", method_refs->length()); + for (int j = 0; j < method_refs->length(); j++) { + Method* method = method_refs->at(j); + + if (!method->on_stack()) { + // no breakpoints for non-running methods + if (method->is_running_emcp()) { + method->set_running_emcp(false); } + } else { + assert (method->is_obsolete() || method->is_running_emcp(), + "emcp method cannot run after emcp bit is cleared"); + log_trace(redefine, class, iklass, purge) + ("purge: %s(%s): prev method @%d in version @%d is alive", + method->name()->as_C_string(), method->signature()->as_C_string(), j, version); } } - // next previous version - last = pv_node; - pv_node = pv_node->previous_versions(); - version++; } - log_trace(redefine, class, iklass, purge) - ("previous version stats: live=%d, deleted=%d", - live_count, deleted_count); + // next previous version + last = pv_node; + pv_node = pv_node->previous_versions(); + version++; } + log_trace(redefine, class, iklass, purge) + ("previous version stats: live=%d, deleted=%d", live_count, deleted_count); } void InstanceKlass::mark_newly_obsolete_methods(Array* old_methods, @@ -3518,8 +3549,8 @@ log_trace(redefine, class, iklass, add) ("adding previous version ref for %s, EMCP_cnt=%d", scratch_class->external_name(), emcp_method_count); - // Clean out old previous versions - purge_previous_versions(this); + // Clean out old previous versions for this class + purge_previous_version_list(); // Mark newly obsolete methods in remaining previous versions. An EMCP method from // a previous redefinition may be made obsolete by this redefinition. @@ -3536,8 +3567,6 @@ // 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; } @@ -3565,12 +3594,12 @@ } // Add previous version if any methods are still running. - log_trace(redefine, class, iklass, add)("scratch class added; one of its methods is on_stack"); + // Set has_previous_version flag for processing during class unloading. + _has_previous_versions = true; + log_trace(redefine, class, iklass, add) ("scratch class added; one of its methods is on_stack."); 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 diff -r 04b19236aa98 -r 59f3c8a69541 hotspot/src/share/vm/oops/instanceKlass.hpp --- a/hotspot/src/share/vm/oops/instanceKlass.hpp Wed Sep 07 15:21:45 2016 +0200 +++ b/hotspot/src/share/vm/oops/instanceKlass.hpp Wed Sep 07 15:25:21 2016 -0400 @@ -709,6 +709,7 @@ // RedefineClasses() support for previous versions: void add_previous_version(instanceKlassHandle ikh, int emcp_method_count); + void purge_previous_version_list(); InstanceKlass* previous_versions() const { return _previous_versions; } #else @@ -768,10 +769,15 @@ } private: - static int _previous_version_count; + static bool _has_previous_versions; public: - static void purge_previous_versions(InstanceKlass* ik); - static bool has_previous_versions() { return _previous_version_count > 0; } + static void purge_previous_versions(InstanceKlass* ik) { + if (ik->has_been_redefined()) { + ik->purge_previous_version_list(); + } + } + + static bool has_previous_versions_and_reset(); // 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) { @@ -792,7 +798,7 @@ #else // INCLUDE_JVMTI static void purge_previous_versions(InstanceKlass* ik) { return; }; - static bool has_previous_versions() { return false; } + static bool has_previous_versions_and_reset() { return false; } void set_cached_class_file(JvmtiCachedClassFileData *data) { assert(data == NULL, "unexpected call with JVMTI disabled"); diff -r 04b19236aa98 -r 59f3c8a69541 hotspot/test/runtime/RedefineTests/RedefinePreviousVersions.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/hotspot/test/runtime/RedefineTests/RedefinePreviousVersions.java Wed Sep 07 15:25:21 2016 -0400 @@ -0,0 +1,120 @@ +/* + * 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 8165246 + * @summary Test has_previous_versions flag and processing during class unloading. + * @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 RedefinePreviousVersions test + */ + +import jdk.test.lib.process.ProcessTools; +import jdk.test.lib.process.OutputAnalyzer; + +public class RedefinePreviousVersions { + + public static String newB = + "class RedefinePreviousVersions$B {" + + "}"; + + static class B { } + + public static String newRunning = + "class RedefinePreviousVersions$Running {" + + " public static volatile boolean stop = true;" + + " static void localSleep() { }" + + " public static void infinite() { }" + + "}"; + + static class Running { + public static volatile boolean stop = false; + static void localSleep() { + try{ + Thread.currentThread().sleep(10);//sleep for 10 ms + } catch(InterruptedException ie) { + } + } + + public static void infinite() { + while (!stop) { localSleep(); } + } + } + + public static void main(String[] args) throws Exception { + + if (args.length > 0) { + + String jarFile = System.getProperty("test.src") + "/testcase.jar"; + + // java -javaagent:redefineagent.jar -Xlog:stuff RedefinePreviousVersions + ProcessBuilder pb = ProcessTools.createJavaProcessBuilder( "-javaagent:redefineagent.jar", + "-Xlog:redefine+class+iklass+add=trace,redefine+class+iklass+purge=trace", + "RedefinePreviousVersions"); + new OutputAnalyzer(pb.start()) + .shouldContain("Class unloading: has_previous_versions = false") + .shouldContain("Class unloading: has_previous_versions = true") + .shouldHaveExitValue(0); + return; + } + + // 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 flag _has_previous_versions should stay false + RedefineClassHelper.redefineClass(B.class, newB); + + for (int i = 0; i < 10 ; i++) { + String s = new String("some garbage"); + System.gc(); + } + + // Start a class that has a method running + new Thread() { + public void run() { + Running.infinite(); + } + }.start(); + + // Since a method of newRunning is running, this class should be added to the previous_version_list + // of Running, and _has_previous_versions should return true at class unloading. + RedefineClassHelper.redefineClass(Running.class, newRunning); + + for (int i = 0; i < 10 ; i++) { + String s = new String("some garbage"); + System.gc(); + } + + // purge should clean everything up, except Xcomp it might not. + Running.stop = true; + + for (int i = 0; i < 10 ; i++) { + String s = new String("some garbage"); + System.gc(); + } + } +}