8026946: JvmtiEnv::SetBreakpoint and JvmtiEnv::ClearBreakpoint should use MethodHandle
8026948: JvmtiEnv::SetBreakpoint and JvmtiEnv::ClearBreakpoint might not work with anonymous classes
Summary: Walk methods in breakpoints for marking on stack so they aren't deallocated by redefine classes. Use class_holder rather than class_loader to keep GC from reclaiming class owning the method.
Reviewed-by: sspitsyn, ehelin, sla
--- a/hotspot/src/share/vm/classfile/metadataOnStackMark.cpp Thu Oct 31 14:11:02 2013 -0400
+++ b/hotspot/src/share/vm/classfile/metadataOnStackMark.cpp Fri Nov 01 10:32:36 2013 -0400
@@ -27,6 +27,7 @@
#include "code/codeCache.hpp"
#include "compiler/compileBroker.hpp"
#include "oops/metadata.hpp"
+#include "prims/jvmtiImpl.hpp"
#include "runtime/synchronizer.hpp"
#include "runtime/thread.hpp"
#include "utilities/growableArray.hpp"
@@ -48,6 +49,7 @@
Threads::metadata_do(Metadata::mark_on_stack);
CodeCache::alive_nmethods_do(nmethod::mark_on_stack);
CompileBroker::mark_on_stack();
+ JvmtiCurrentBreakpoints::metadata_do(Metadata::mark_on_stack);
}
MetadataOnStackMark::~MetadataOnStackMark() {
--- a/hotspot/src/share/vm/prims/jvmtiImpl.cpp Thu Oct 31 14:11:02 2013 -0400
+++ b/hotspot/src/share/vm/prims/jvmtiImpl.cpp Fri Nov 01 10:32:36 2013 -0400
@@ -210,6 +210,14 @@
}
}
+void GrowableCache::metadata_do(void f(Metadata*)) {
+ int len = _elements->length();
+ for (int i=0; i<len; i++) {
+ GrowableElement *e = _elements->at(i);
+ e->metadata_do(f);
+ }
+}
+
void GrowableCache::gc_epilogue() {
int len = _elements->length();
for (int i=0; i<len; i++) {
@@ -224,20 +232,20 @@
JvmtiBreakpoint::JvmtiBreakpoint() {
_method = NULL;
_bci = 0;
- _class_loader = NULL;
+ _class_holder = NULL;
}
JvmtiBreakpoint::JvmtiBreakpoint(Method* m_method, jlocation location) {
_method = m_method;
- _class_loader = _method->method_holder()->class_loader_data()->class_loader();
+ _class_holder = _method->method_holder()->klass_holder();
#ifdef CHECK_UNHANDLED_OOPS
- // _class_loader can't be wrapped in a Handle, because JvmtiBreakpoint:s are
- // eventually allocated on the heap.
+ // _class_holder can't be wrapped in a Handle, because JvmtiBreakpoints are
+ // sometimes allocated on the heap.
//
- // The code handling JvmtiBreakpoint:s allocated on the stack can't be
- // interrupted by a GC until _class_loader is reachable by the GC via the
+ // The code handling JvmtiBreakpoints allocated on the stack can't be
+ // interrupted by a GC until _class_holder is reachable by the GC via the
// oops_do method.
- Thread::current()->allow_unhandled_oop(&_class_loader);
+ Thread::current()->allow_unhandled_oop(&_class_holder);
#endif // CHECK_UNHANDLED_OOPS
assert(_method != NULL, "_method != NULL");
_bci = (int) location;
@@ -247,7 +255,7 @@
void JvmtiBreakpoint::copy(JvmtiBreakpoint& bp) {
_method = bp._method;
_bci = bp._bci;
- _class_loader = bp._class_loader;
+ _class_holder = bp._class_holder;
}
bool JvmtiBreakpoint::lessThan(JvmtiBreakpoint& bp) {
@@ -365,6 +373,13 @@
}
}
+void VM_ChangeBreakpoints::metadata_do(void f(Metadata*)) {
+ // Walk metadata in breakpoints to keep from being deallocated with RedefineClasses
+ if (_bp != NULL) {
+ _bp->metadata_do(f);
+ }
+}
+
//
// class JvmtiBreakpoints
//
@@ -381,6 +396,10 @@
_bps.oops_do(f);
}
+void JvmtiBreakpoints::metadata_do(void f(Metadata*)) {
+ _bps.metadata_do(f);
+}
+
void JvmtiBreakpoints::gc_epilogue() {
_bps.gc_epilogue();
}
@@ -499,6 +518,12 @@
}
}
+void JvmtiCurrentBreakpoints::metadata_do(void f(Metadata*)) {
+ if (_jvmti_breakpoints != NULL) {
+ _jvmti_breakpoints->metadata_do(f);
+ }
+}
+
void JvmtiCurrentBreakpoints::gc_epilogue() {
if (_jvmti_breakpoints != NULL) {
_jvmti_breakpoints->gc_epilogue();
--- a/hotspot/src/share/vm/prims/jvmtiImpl.hpp Thu Oct 31 14:11:02 2013 -0400
+++ b/hotspot/src/share/vm/prims/jvmtiImpl.hpp Fri Nov 01 10:32:36 2013 -0400
@@ -69,6 +69,7 @@
virtual bool lessThan(GrowableElement *e)=0;
virtual GrowableElement *clone() =0;
virtual void oops_do(OopClosure* f) =0;
+ virtual void metadata_do(void f(Metadata*)) =0;
};
class GrowableCache VALUE_OBJ_CLASS_SPEC {
@@ -115,6 +116,8 @@
void clear();
// apply f to every element and update the cache
void oops_do(OopClosure* f);
+ // walk metadata to preserve for RedefineClasses
+ void metadata_do(void f(Metadata*));
// update the cache after a full gc
void gc_epilogue();
};
@@ -148,6 +151,7 @@
void remove (int index) { _cache.remove(index); }
void clear() { _cache.clear(); }
void oops_do(OopClosure* f) { _cache.oops_do(f); }
+ void metadata_do(void f(Metadata*)) { _cache.metadata_do(f); }
void gc_epilogue() { _cache.gc_epilogue(); }
};
@@ -169,7 +173,7 @@
Method* _method;
int _bci;
Bytecodes::Code _orig_bytecode;
- oop _class_loader;
+ oop _class_holder; // keeps _method memory from being deallocated
public:
JvmtiBreakpoint();
@@ -191,9 +195,15 @@
bool lessThan(GrowableElement* e) { Unimplemented(); return false; }
bool equals(GrowableElement* e) { return equals((JvmtiBreakpoint&) *e); }
void oops_do(OopClosure* f) {
- // Mark the method loader as live
- f->do_oop(&_class_loader);
+ // Mark the method loader as live so the Method* class loader doesn't get
+ // unloaded and Method* memory reclaimed.
+ f->do_oop(&_class_holder);
}
+ void metadata_do(void f(Metadata*)) {
+ // walk metadata to preserve for RedefineClasses
+ f(_method);
+ }
+
GrowableElement *clone() {
JvmtiBreakpoint *bp = new JvmtiBreakpoint();
bp->copy(*this);
@@ -239,6 +249,7 @@
int length();
void oops_do(OopClosure* f);
+ void metadata_do(void f(Metadata*));
void print();
int set(JvmtiBreakpoint& bp);
@@ -288,6 +299,7 @@
static inline bool is_breakpoint(address bcp);
static void oops_do(OopClosure* f);
+ static void metadata_do(void f(Metadata*));
static void gc_epilogue();
};
@@ -332,6 +344,7 @@
VMOp_Type type() const { return VMOp_ChangeBreakpoints; }
void doit();
void oops_do(OopClosure* f);
+ void metadata_do(void f(Metadata*));
};