8009728: nsk/jvmti/AttachOnDemand/attach030 crashes on Win32
Summary: ActiveMethodOopsCache was used to keep track of old versions of some methods that are cached in Universe but is buggy with permgen removal and not needed anymore
Reviewed-by: sspitsyn, dcubed, mseledtsov
--- a/hotspot/src/share/vm/memory/universe.cpp Sat Aug 10 10:56:27 2013 -0700
+++ b/hotspot/src/share/vm/memory/universe.cpp Mon Aug 12 17:24:54 2013 -0400
@@ -105,10 +105,9 @@
Array<Klass*>* Universe::_the_array_interfaces_array = NULL;
oop Universe::_the_null_string = NULL;
oop Universe::_the_min_jint_string = NULL;
-LatestMethodOopCache* Universe::_finalizer_register_cache = NULL;
-LatestMethodOopCache* Universe::_loader_addClass_cache = NULL;
-LatestMethodOopCache* Universe::_pd_implies_cache = NULL;
-ActiveMethodOopsCache* Universe::_reflect_invoke_cache = NULL;
+LatestMethodCache* Universe::_finalizer_register_cache = NULL;
+LatestMethodCache* Universe::_loader_addClass_cache = NULL;
+LatestMethodCache* Universe::_pd_implies_cache = NULL;
oop Universe::_out_of_memory_error_java_heap = NULL;
oop Universe::_out_of_memory_error_metaspace = NULL;
oop Universe::_out_of_memory_error_class_metaspace = NULL;
@@ -225,7 +224,6 @@
f->do_ptr((void**)&_the_empty_klass_array);
_finalizer_register_cache->serialize(f);
_loader_addClass_cache->serialize(f);
- _reflect_invoke_cache->serialize(f);
_pd_implies_cache->serialize(f);
}
@@ -649,10 +647,9 @@
// We have a heap so create the Method* caches before
// Metaspace::initialize_shared_spaces() tries to populate them.
- Universe::_finalizer_register_cache = new LatestMethodOopCache();
- Universe::_loader_addClass_cache = new LatestMethodOopCache();
- Universe::_pd_implies_cache = new LatestMethodOopCache();
- Universe::_reflect_invoke_cache = new ActiveMethodOopsCache();
+ Universe::_finalizer_register_cache = new LatestMethodCache();
+ Universe::_loader_addClass_cache = new LatestMethodCache();
+ Universe::_pd_implies_cache = new LatestMethodCache();
if (UseSharedSpaces) {
// Read the data structures supporting the shared spaces (shared
@@ -1088,35 +1085,21 @@
vmSymbols::register_method_name(),
vmSymbols::register_method_signature());
if (m == NULL || !m->is_static()) {
- THROW_MSG_(vmSymbols::java_lang_NoSuchMethodException(),
- "java.lang.ref.Finalizer.register", false);
+ tty->print_cr("Unable to link/verify Finalizer.register method");
+ return false; // initialization failed (cannot throw exception yet)
}
Universe::_finalizer_register_cache->init(
- SystemDictionary::Finalizer_klass(), m, CHECK_false);
-
- // Resolve on first use and initialize class.
- // Note: No race-condition here, since a resolve will always return the same result
-
- // Setup method for security checks
- k = SystemDictionary::resolve_or_fail(vmSymbols::java_lang_reflect_Method(), true, CHECK_false);
- k_h = instanceKlassHandle(THREAD, k);
- k_h->link_class(CHECK_false);
- m = k_h->find_method(vmSymbols::invoke_name(), vmSymbols::object_object_array_object_signature());
- if (m == NULL || m->is_static()) {
- THROW_MSG_(vmSymbols::java_lang_NoSuchMethodException(),
- "java.lang.reflect.Method.invoke", false);
- }
- Universe::_reflect_invoke_cache->init(k_h(), m, CHECK_false);
+ SystemDictionary::Finalizer_klass(), m);
// Setup method for registering loaded classes in class loader vector
InstanceKlass::cast(SystemDictionary::ClassLoader_klass())->link_class(CHECK_false);
m = InstanceKlass::cast(SystemDictionary::ClassLoader_klass())->find_method(vmSymbols::addClass_name(), vmSymbols::class_void_signature());
if (m == NULL || m->is_static()) {
- THROW_MSG_(vmSymbols::java_lang_NoSuchMethodException(),
- "java.lang.ClassLoader.addClass", false);
+ tty->print_cr("Unable to link/verify ClassLoader.addClass method");
+ return false; // initialization failed (cannot throw exception yet)
}
Universe::_loader_addClass_cache->init(
- SystemDictionary::ClassLoader_klass(), m, CHECK_false);
+ SystemDictionary::ClassLoader_klass(), m);
// Setup method for checking protection domain
InstanceKlass::cast(SystemDictionary::ProtectionDomain_klass())->link_class(CHECK_false);
@@ -1132,7 +1115,7 @@
return false; // initialization failed
}
Universe::_pd_implies_cache->init(
- SystemDictionary::ProtectionDomain_klass(), m, CHECK_false);;
+ SystemDictionary::ProtectionDomain_klass(), m);;
}
// The folowing is initializing converter functions for serialization in
@@ -1455,7 +1438,7 @@
}
-void CommonMethodOopCache::init(Klass* k, Method* m, TRAPS) {
+void LatestMethodCache::init(Klass* k, Method* m) {
if (!UseSharedSpaces) {
_klass = k;
}
@@ -1471,88 +1454,7 @@
}
-ActiveMethodOopsCache::~ActiveMethodOopsCache() {
- if (_prev_methods != NULL) {
- delete _prev_methods;
- _prev_methods = NULL;
- }
-}
-
-
-void ActiveMethodOopsCache::add_previous_version(Method* method) {
- assert(Thread::current()->is_VM_thread(),
- "only VMThread can add previous versions");
-
- // Only append the previous method if it is executing on the stack.
- if (method->on_stack()) {
-
- if (_prev_methods == NULL) {
- // This is the first previous version so make some space.
- // Start with 2 elements under the assumption that the class
- // won't be redefined much.
- _prev_methods = new (ResourceObj::C_HEAP, mtClass) GrowableArray<Method*>(2, true);
- }
-
- // RC_TRACE macro has an embedded ResourceMark
- RC_TRACE(0x00000100,
- ("add: %s(%s): adding prev version ref for cached method @%d",
- method->name()->as_C_string(), method->signature()->as_C_string(),
- _prev_methods->length()));
-
- _prev_methods->append(method);
- }
-
-
- // Since the caller is the VMThread and we are at a safepoint, this is a good
- // time to clear out unused method references.
-
- if (_prev_methods == NULL) return;
-
- for (int i = _prev_methods->length() - 1; i >= 0; i--) {
- Method* method = _prev_methods->at(i);
- assert(method != NULL, "weak method ref was unexpectedly cleared");
-
- if (!method->on_stack()) {
- // This method isn't running anymore so remove it
- _prev_methods->remove_at(i);
- MetadataFactory::free_metadata(method->method_holder()->class_loader_data(), method);
- } else {
- // RC_TRACE macro has an embedded ResourceMark
- RC_TRACE(0x00000400,
- ("add: %s(%s): previous cached method @%d is alive",
- method->name()->as_C_string(), method->signature()->as_C_string(), i));
- }
- }
-} // end add_previous_version()
-
-
-bool ActiveMethodOopsCache::is_same_method(const Method* method) const {
- InstanceKlass* ik = InstanceKlass::cast(klass());
- const Method* check_method = ik->method_with_idnum(method_idnum());
- assert(check_method != NULL, "sanity check");
- if (check_method == method) {
- // done with the easy case
- return true;
- }
-
- if (_prev_methods != NULL) {
- // The cached method has been redefined at least once so search
- // the previous versions for a match.
- for (int i = 0; i < _prev_methods->length(); i++) {
- check_method = _prev_methods->at(i);
- if (check_method == method) {
- // a previous version matches
- return true;
- }
- }
- }
-
- // either no previous versions or no previous version matched
- return false;
-}
-
-
-Method* LatestMethodOopCache::get_Method() {
+Method* LatestMethodCache::get_method() {
if (klass() == NULL) return NULL;
InstanceKlass* ik = InstanceKlass::cast(klass());
Method* m = ik->method_with_idnum(method_idnum());
--- a/hotspot/src/share/vm/memory/universe.hpp Sat Aug 10 10:56:27 2013 -0700
+++ b/hotspot/src/share/vm/memory/universe.hpp Mon Aug 12 17:24:54 2013 -0400
@@ -41,10 +41,11 @@
class DeferredObjAllocEvent;
-// Common parts of a Method* cache. This cache safely interacts with
-// the RedefineClasses API.
-//
-class CommonMethodOopCache : public CHeapObj<mtClass> {
+// A helper class for caching a Method* when the user of the cache
+// only cares about the latest version of the Method*. This cache safely
+// interacts with the RedefineClasses API.
+
+class LatestMethodCache : public CHeapObj<mtClass> {
// We save the Klass* and the idnum of Method* in order to get
// the current cached Method*.
private:
@@ -52,12 +53,14 @@
int _method_idnum;
public:
- CommonMethodOopCache() { _klass = NULL; _method_idnum = -1; }
- ~CommonMethodOopCache() { _klass = NULL; _method_idnum = -1; }
+ LatestMethodCache() { _klass = NULL; _method_idnum = -1; }
+ ~LatestMethodCache() { _klass = NULL; _method_idnum = -1; }
- void init(Klass* k, Method* m, TRAPS);
- Klass* klass() const { return _klass; }
- int method_idnum() const { return _method_idnum; }
+ void init(Klass* k, Method* m);
+ Klass* klass() const { return _klass; }
+ int method_idnum() const { return _method_idnum; }
+
+ Method* get_method();
// Enhanced Class Redefinition support
void classes_do(void f(Klass*)) {
@@ -72,39 +75,6 @@
};
-// A helper class for caching a Method* when the user of the cache
-// cares about all versions of the Method*.
-//
-class ActiveMethodOopsCache : public CommonMethodOopCache {
- // This subclass adds weak references to older versions of the
- // Method* and a query method for a Method*.
-
- private:
- // If the cached Method* has not been redefined, then
- // _prev_methods will be NULL. If all of the previous
- // versions of the method have been collected, then
- // _prev_methods can have a length of zero.
- GrowableArray<Method*>* _prev_methods;
-
- public:
- ActiveMethodOopsCache() { _prev_methods = NULL; }
- ~ActiveMethodOopsCache();
-
- void add_previous_version(Method* method);
- bool is_same_method(const Method* method) const;
-};
-
-
-// A helper class for caching a Method* when the user of the cache
-// only cares about the latest version of the Method*.
-//
-class LatestMethodOopCache : public CommonMethodOopCache {
- // This subclass adds a getter method for the latest Method*.
-
- public:
- Method* get_Method();
-};
-
// For UseCompressedOops and UseCompressedKlassPointers.
struct NarrowPtrStruct {
// Base address for oop/klass-within-java-object materialization.
@@ -174,10 +144,10 @@
static objArrayOop _the_empty_class_klass_array; // Canonicalized obj array of type java.lang.Class
static oop _the_null_string; // A cache of "null" as a Java string
static oop _the_min_jint_string; // A cache of "-2147483648" as a Java string
- static LatestMethodOopCache* _finalizer_register_cache; // static method for registering finalizable objects
- static LatestMethodOopCache* _loader_addClass_cache; // method for registering loaded classes in class loader vector
- static LatestMethodOopCache* _pd_implies_cache; // method for checking protection domain attributes
- static ActiveMethodOopsCache* _reflect_invoke_cache; // method for security checks
+ static LatestMethodCache* _finalizer_register_cache; // static method for registering finalizable objects
+ static LatestMethodCache* _loader_addClass_cache; // method for registering loaded classes in class loader vector
+ static LatestMethodCache* _pd_implies_cache; // method for checking protection domain attributes
+
// preallocated error objects (no backtrace)
static oop _out_of_memory_error_java_heap;
static oop _out_of_memory_error_metaspace;
@@ -334,11 +304,11 @@
static Array<Klass*>* the_array_interfaces_array() { return _the_array_interfaces_array; }
static oop the_null_string() { return _the_null_string; }
static oop the_min_jint_string() { return _the_min_jint_string; }
- static Method* finalizer_register_method() { return _finalizer_register_cache->get_Method(); }
- static Method* loader_addClass_method() { return _loader_addClass_cache->get_Method(); }
- static Method* protection_domain_implies_method() { return _pd_implies_cache->get_Method(); }
- static ActiveMethodOopsCache* reflect_invoke_cache() { return _reflect_invoke_cache; }
+ static Method* finalizer_register_method() { return _finalizer_register_cache->get_method(); }
+ static Method* loader_addClass_method() { return _loader_addClass_cache->get_method(); }
+
+ static Method* protection_domain_implies_method() { return _pd_implies_cache->get_method(); }
static oop null_ptr_exception_instance() { return _null_ptr_exception_instance; }
static oop arithmetic_exception_instance() { return _arithmetic_exception_instance; }
--- a/hotspot/src/share/vm/oops/method.cpp Sat Aug 10 10:56:27 2013 -0700
+++ b/hotspot/src/share/vm/oops/method.cpp Mon Aug 12 17:24:54 2013 -0400
@@ -981,7 +981,6 @@
bool Method::is_ignored_by_security_stack_walk() const {
const bool use_new_reflection = JDK_Version::is_gte_jdk14x_version() && UseNewReflection;
- assert(intrinsic_id() != vmIntrinsics::_invoke || Universe::reflect_invoke_cache()->is_same_method((Method*)this), "sanity");
if (intrinsic_id() == vmIntrinsics::_invoke) {
// This is Method.invoke() -- ignore it
return true;
--- a/hotspot/src/share/vm/prims/jvmtiRedefineClasses.cpp Sat Aug 10 10:56:27 2013 -0700
+++ b/hotspot/src/share/vm/prims/jvmtiRedefineClasses.cpp Mon Aug 12 17:24:54 2013 -0400
@@ -3217,15 +3217,6 @@
JvmtiBreakpoints& jvmti_breakpoints = JvmtiCurrentBreakpoints::get_jvmti_breakpoints();
jvmti_breakpoints.clearall_in_class_at_safepoint(the_class_oop);
- if (the_class_oop == Universe::reflect_invoke_cache()->klass()) {
- // We are redefining java.lang.reflect.Method. Method.invoke() is
- // cached and users of the cache care about each active version of
- // the method so we have to track this previous version.
- // Do this before methods get switched
- Universe::reflect_invoke_cache()->add_previous_version(
- the_class->method_with_idnum(Universe::reflect_invoke_cache()->method_idnum()));
- }
-
// Deoptimize all compiled code that depends on this class
flush_dependent_code(the_class, THREAD);
--- a/hotspot/test/runtime/RedefineObject/Agent.java Sat Aug 10 10:56:27 2013 -0700
+++ b/hotspot/test/runtime/RedefineObject/Agent.java Mon Aug 12 17:24:54 2013 -0400
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2013, 2013, 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
@@ -22,6 +22,7 @@
*/
import java.security.*;
import java.lang.instrument.*;
+import java.lang.reflect.*;
public class Agent implements ClassFileTransformer {
public synchronized byte[] transform(final ClassLoader classLoader,
@@ -29,22 +30,34 @@
Class<?> classBeingRedefined,
ProtectionDomain protectionDomain,
byte[] classfileBuffer) {
- //System.out.println("Transforming class " + className);
+ System.out.println("Transforming class " + className);
return classfileBuffer;
}
+ public static void redefine(String agentArgs, Instrumentation instrumentation, Class to_redefine) {
+
+ try {
+ instrumentation.retransformClasses(to_redefine);
+ } catch (Exception e) {
+ e.printStackTrace();
+ }
+
+ }
+
public static void premain(String agentArgs, Instrumentation instrumentation) {
-
Agent transformer = new Agent();
-
instrumentation.addTransformer(transformer, true);
- Class c = Object.class;
- try {
- instrumentation.retransformClasses(c);
- } catch (Exception e) {
- e.printStackTrace();
- }
+ // Redefine java/lang/Object and java/lang/reflect/Method.invoke and
+ // java/lang/ClassLoader
+ Class object_class = Object.class;
+ redefine(agentArgs, instrumentation, object_class);
+
+ Class method_class = Method.class;
+ redefine(agentArgs, instrumentation, method_class);
+
+ Class loader_class = ClassLoader.class;
+ redefine(agentArgs, instrumentation, loader_class);
instrumentation.removeTransformer(transformer);
}
@@ -57,5 +70,14 @@
System.gc();
ba.clone();
}
+ try {
+ // Use java/lang/reflect/Method.invoke to call
+ WalkThroughInvoke a = new WalkThroughInvoke();
+ Class aclass = WalkThroughInvoke.class;
+ Method m = aclass.getMethod("stackWalk");
+ m.invoke(a);
+ } catch (Exception x) {
+ x.printStackTrace();
+ }
}
}
--- a/hotspot/test/runtime/RedefineObject/TestRedefineObject.java Sat Aug 10 10:56:27 2013 -0700
+++ b/hotspot/test/runtime/RedefineObject/TestRedefineObject.java Mon Aug 12 17:24:54 2013 -0400
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2013, 2013, 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
@@ -26,14 +26,17 @@
/*
* Test to redefine java/lang/Object and verify that it doesn't crash on vtable
* call on basic array type.
+ * Test to redefine java/lang/ClassLoader and java/lang/reflect/Method to make
+ * sure cached versions used afterward are the current version.
*
* @test
* @bug 8005056
+ * @bug 8009728
* @library /testlibrary
* @build Agent
* @run main ClassFileInstaller Agent
* @run main TestRedefineObject
- * @run main/othervm -javaagent:agent.jar Agent
+ * @run main/othervm -javaagent:agent.jar -XX:TraceRedefineClasses=5 Agent
*/
public class TestRedefineObject {
public static void main(String[] args) throws Exception {
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/runtime/RedefineObject/WalkThroughInvoke.java Mon Aug 12 17:24:54 2013 -0400
@@ -0,0 +1,38 @@
+/*
+ * Copyright (c) 2013, 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.
+ */
+import java.lang.reflect.*;
+
+public class WalkThroughInvoke {
+ public void stackWalk() {
+ try {
+ Class b = Object.class;
+ SecurityManager sm = new SecurityManager();
+ // Walks the stack with Method.invoke in the stack (which is the
+ // purpose of the test) before it gets an AccessControlException.
+ sm.checkMemberAccess(b, Member.DECLARED);
+ } catch (java.security.AccessControlException e) {
+ // Ignoring an 'AccessControlException' exception since
+ // it is expected as part of this test.
+ }
+ }
+};