6497639: 4/3 Profiling Swing application caused JVM crash
Summary: Make RedefineClasses() interoperate better with class sharing.
Reviewed-by: sspitsyn, jmasa
--- a/hotspot/src/share/vm/classfile/dictionary.cpp Wed Jul 05 16:34:33 2017 +0200
+++ b/hotspot/src/share/vm/classfile/dictionary.cpp Wed Mar 12 18:06:50 2008 -0700
@@ -155,8 +155,8 @@
for (int i = ik->previous_versions()->length() - 1; i >= 0; i--) {
// check the previous versions array for GC'ed weak refs
PreviousVersionNode * pv_node = ik->previous_versions()->at(i);
- jweak cp_ref = pv_node->prev_constant_pool();
- assert(cp_ref != NULL, "weak cp ref was unexpectedly cleared");
+ jobject cp_ref = pv_node->prev_constant_pool();
+ assert(cp_ref != NULL, "cp ref was unexpectedly cleared");
if (cp_ref == NULL) {
delete pv_node;
ik->previous_versions()->remove_at(i);
--- a/hotspot/src/share/vm/memory/compactingPermGenGen.cpp Wed Jul 05 16:34:33 2017 +0200
+++ b/hotspot/src/share/vm/memory/compactingPermGenGen.cpp Wed Mar 12 18:06:50 2008 -0700
@@ -26,9 +26,27 @@
#include "incls/_compactingPermGenGen.cpp.incl"
-// Recursively adjust all pointers in an object and all objects by
-// referenced it. Clear marks on objects in order to prevent visiting
-// any object twice.
+// An ObjectClosure helper: Recursively adjust all pointers in an object
+// and all objects by referenced it. Clear marks on objects in order to
+// prevent visiting any object twice. This helper is used when the
+// RedefineClasses() API has been called.
+
+class AdjustSharedObjectClosure : public ObjectClosure {
+public:
+ void do_object(oop obj) {
+ if (obj->is_shared_readwrite()) {
+ if (obj->mark()->is_marked()) {
+ obj->init_mark(); // Don't revisit this object.
+ obj->adjust_pointers(); // Adjust this object's references.
+ }
+ }
+ }
+};
+
+
+// An OopClosure helper: Recursively adjust all pointers in an object
+// and all objects by referenced it. Clear marks on objects in order
+// to prevent visiting any object twice.
class RecursiveAdjustSharedObjectClosure : public OopClosure {
public:
@@ -274,15 +292,34 @@
// objects in the space will page in more objects than we need.
// Instead, use the system dictionary as strong roots into the read
// write space.
+//
+// If a RedefineClasses() call has been made, then we have to iterate
+// over the entire shared read-write space in order to find all the
+// objects that need to be forwarded. For example, it is possible for
+// an nmethod to be found and marked in GC phase-1 only for the nmethod
+// to be freed by the time we reach GC phase-3. The underlying method
+// is still marked, but we can't (easily) find it in GC phase-3 so we
+// blow up in GC phase-4. With RedefineClasses() we want replaced code
+// (EMCP or obsolete) to go away (i.e., be collectible) once it is no
+// longer being executed by any thread so we keep minimal attachments
+// to the replaced code. However, we can't guarantee when those EMCP
+// or obsolete methods will be collected so they may still be out there
+// even after we've severed our minimal attachments.
void CompactingPermGenGen::pre_adjust_pointers() {
if (spec()->enable_shared_spaces()) {
- RecursiveAdjustSharedObjectClosure blk;
- Universe::oops_do(&blk);
- StringTable::oops_do(&blk);
- SystemDictionary::always_strong_classes_do(&blk);
- TraversePlaceholdersClosure tpc;
- SystemDictionary::placeholders_do(&tpc);
+ if (JvmtiExport::has_redefined_a_class()) {
+ // RedefineClasses() requires a brute force approach
+ AdjustSharedObjectClosure blk;
+ rw_space()->object_iterate(&blk);
+ } else {
+ RecursiveAdjustSharedObjectClosure blk;
+ Universe::oops_do(&blk);
+ StringTable::oops_do(&blk);
+ SystemDictionary::always_strong_classes_do(&blk);
+ TraversePlaceholdersClosure tpc;
+ SystemDictionary::placeholders_do(&tpc);
+ }
}
}
--- a/hotspot/src/share/vm/oops/instanceKlass.cpp Wed Jul 05 16:34:33 2017 +0200
+++ b/hotspot/src/share/vm/oops/instanceKlass.cpp Wed Mar 12 18:06:50 2008 -0700
@@ -2165,12 +2165,20 @@
RC_TRACE(0x00000100, ("adding previous version ref for %s @%d, EMCP_cnt=%d",
ikh->external_name(), _previous_versions->length(), emcp_method_count));
constantPoolHandle cp_h(ikh->constants());
- jweak cp_ref = JNIHandles::make_weak_global(cp_h);
+ jobject cp_ref;
+ if (cp_h->is_shared()) {
+ // a shared ConstantPool requires a regular reference; a weak
+ // reference would be collectible
+ cp_ref = JNIHandles::make_global(cp_h);
+ } else {
+ cp_ref = JNIHandles::make_weak_global(cp_h);
+ }
PreviousVersionNode * pv_node = NULL;
objArrayOop old_methods = ikh->methods();
if (emcp_method_count == 0) {
- pv_node = new PreviousVersionNode(cp_ref, NULL);
+ // non-shared ConstantPool gets a weak reference
+ pv_node = new PreviousVersionNode(cp_ref, !cp_h->is_shared(), NULL);
RC_TRACE(0x00000400,
("add: all methods are obsolete; flushing any EMCP weak refs"));
} else {
@@ -2190,7 +2198,8 @@
}
}
}
- pv_node = new PreviousVersionNode(cp_ref, method_refs);
+ // non-shared ConstantPool gets a weak reference
+ pv_node = new PreviousVersionNode(cp_ref, !cp_h->is_shared(), method_refs);
}
_previous_versions->append(pv_node);
@@ -2208,7 +2217,7 @@
// check the previous versions array for a GC'ed weak refs
pv_node = _previous_versions->at(i);
cp_ref = pv_node->prev_constant_pool();
- assert(cp_ref != NULL, "weak cp ref was unexpectedly cleared");
+ assert(cp_ref != NULL, "cp ref was unexpectedly cleared");
if (cp_ref == NULL) {
delete pv_node;
_previous_versions->remove_at(i);
@@ -2281,7 +2290,7 @@
// check the previous versions array for a GC'ed weak refs
pv_node = _previous_versions->at(j);
cp_ref = pv_node->prev_constant_pool();
- assert(cp_ref != NULL, "weak cp ref was unexpectedly cleared");
+ assert(cp_ref != NULL, "cp ref was unexpectedly cleared");
if (cp_ref == NULL) {
delete pv_node;
_previous_versions->remove_at(j);
@@ -2379,8 +2388,8 @@
// been GC'ed
PreviousVersionNode * pv_node = _previous_versions->at(i);
- jweak cp_ref = pv_node->prev_constant_pool();
- assert(cp_ref != NULL, "weak reference was unexpectedly cleared");
+ jobject cp_ref = pv_node->prev_constant_pool();
+ assert(cp_ref != NULL, "cp reference was unexpectedly cleared");
if (cp_ref == NULL) {
continue; // robustness
}
@@ -2440,10 +2449,11 @@
// Construct a PreviousVersionNode entry for the array hung off
// the instanceKlass.
-PreviousVersionNode::PreviousVersionNode(jweak prev_constant_pool,
- GrowableArray<jweak>* prev_EMCP_methods) {
+PreviousVersionNode::PreviousVersionNode(jobject prev_constant_pool,
+ bool prev_cp_is_weak, GrowableArray<jweak>* prev_EMCP_methods) {
_prev_constant_pool = prev_constant_pool;
+ _prev_cp_is_weak = prev_cp_is_weak;
_prev_EMCP_methods = prev_EMCP_methods;
}
@@ -2451,7 +2461,11 @@
// Destroy a PreviousVersionNode
PreviousVersionNode::~PreviousVersionNode() {
if (_prev_constant_pool != NULL) {
- JNIHandles::destroy_weak_global(_prev_constant_pool);
+ if (_prev_cp_is_weak) {
+ JNIHandles::destroy_weak_global(_prev_constant_pool);
+ } else {
+ JNIHandles::destroy_global(_prev_constant_pool);
+ }
}
if (_prev_EMCP_methods != NULL) {
@@ -2471,8 +2485,8 @@
_prev_constant_pool_handle = constantPoolHandle(); // NULL handle
_prev_EMCP_method_handles = NULL;
- jweak cp_ref = pv_node->prev_constant_pool();
- assert(cp_ref != NULL, "weak constant pool ref was unexpectedly cleared");
+ jobject cp_ref = pv_node->prev_constant_pool();
+ assert(cp_ref != NULL, "constant pool ref was unexpectedly cleared");
if (cp_ref == NULL) {
return; // robustness
}
--- a/hotspot/src/share/vm/oops/instanceKlass.hpp Wed Jul 05 16:34:33 2017 +0200
+++ b/hotspot/src/share/vm/oops/instanceKlass.hpp Wed Mar 12 18:06:50 2008 -0700
@@ -838,11 +838,20 @@
// A collection point for interesting information about the previous
// version(s) of an instanceKlass. This class uses weak references to
// the information so that the information may be collected as needed
-// by the system. A GrowableArray of PreviousVersionNodes is attached
+// by the system. If the information is shared, then a regular
+// reference must be used because a weak reference would be seen as
+// collectible. A GrowableArray of PreviousVersionNodes is attached
// to the instanceKlass as needed. See PreviousVersionWalker below.
class PreviousVersionNode : public CHeapObj {
private:
- jweak _prev_constant_pool;
+ // A shared ConstantPool is never collected so we'll always have
+ // a reference to it so we can update items in the cache. We'll
+ // have a weak reference to a non-shared ConstantPool until all
+ // of the methods (EMCP or obsolete) have been collected; the
+ // non-shared ConstantPool becomes collectible at that point.
+ jobject _prev_constant_pool; // regular or weak reference
+ bool _prev_cp_is_weak; // true if not a shared ConstantPool
+
// If the previous version of the instanceKlass doesn't have any
// EMCP methods, then _prev_EMCP_methods will be NULL. If all the
// EMCP methods have been collected, then _prev_EMCP_methods can
@@ -850,10 +859,10 @@
GrowableArray<jweak>* _prev_EMCP_methods;
public:
- PreviousVersionNode(jweak prev_constant_pool,
+ PreviousVersionNode(jobject prev_constant_pool, bool prev_cp_is_weak,
GrowableArray<jweak>* prev_EMCP_methods);
~PreviousVersionNode();
- jweak prev_constant_pool() const {
+ jobject prev_constant_pool() const {
return _prev_constant_pool;
}
GrowableArray<jweak>* prev_EMCP_methods() const {