7122253: Instrumentation.retransformClasses() leaks class bytes
Summary: Change ClassFileParser::parseClassFile() to use the instanceKlass:_cached_class_file_bytes field to avoid leaking the cache.
Reviewed-by: coleenp, acorn, poonam
--- a/hotspot/src/share/vm/classfile/classFileParser.cpp Wed Dec 21 18:22:14 2011 -0800
+++ b/hotspot/src/share/vm/classfile/classFileParser.cpp Thu Dec 22 12:50:42 2011 -0800
@@ -45,6 +45,7 @@
#include "oops/methodOop.hpp"
#include "oops/symbol.hpp"
#include "prims/jvmtiExport.hpp"
+#include "prims/jvmtiThreadState.hpp"
#include "runtime/javaCalls.hpp"
#include "runtime/perfData.hpp"
#include "runtime/reflection.hpp"
@@ -2639,8 +2640,11 @@
TempNewSymbol& parsed_name,
bool verify,
TRAPS) {
- // So that JVMTI can cache class file in the state before retransformable agents
- // have modified it
+ // When a retransformable agent is attached, JVMTI caches the
+ // class bytes that existed before the first retransformation.
+ // If RedefineClasses() was used before the retransformable
+ // agent attached, then the cached class bytes may not be the
+ // original class bytes.
unsigned char *cached_class_file_bytes = NULL;
jint cached_class_file_length;
@@ -2660,6 +2664,20 @@
_max_bootstrap_specifier_index = -1;
if (JvmtiExport::should_post_class_file_load_hook()) {
+ // Get the cached class file bytes (if any) from the
+ // class that is being redefined.
+ JvmtiThreadState *state = JvmtiThreadState::state_for(jt);
+ KlassHandle *h_class_being_redefined =
+ state->get_class_being_redefined();
+ if (h_class_being_redefined != NULL) {
+ instanceKlassHandle ikh_class_being_redefined =
+ instanceKlassHandle(THREAD, (*h_class_being_redefined)());
+ cached_class_file_bytes =
+ ikh_class_being_redefined->get_cached_class_file_bytes();
+ cached_class_file_length =
+ ikh_class_being_redefined->get_cached_class_file_len();
+ }
+
unsigned char* ptr = cfs->buffer();
unsigned char* end_ptr = cfs->buffer() + cfs->length();
--- a/hotspot/src/share/vm/prims/jvmtiEnv.cpp Wed Dec 21 18:22:14 2011 -0800
+++ b/hotspot/src/share/vm/prims/jvmtiEnv.cpp Thu Dec 22 12:50:42 2011 -0800
@@ -267,7 +267,10 @@
instanceKlassHandle ikh(current_thread, k_oop);
if (ikh->get_cached_class_file_bytes() == NULL) {
- // not cached, we need to reconstitute the class file from VM representation
+ // Not cached, we need to reconstitute the class file from the
+ // VM representation. We don't attach the reconstituted class
+ // bytes to the instanceKlass here because they have not been
+ // validated and we're not at a safepoint.
constantPoolHandle constants(current_thread, ikh->constants());
ObjectLocker ol(constants, current_thread); // lock constant pool while we query it
--- a/hotspot/src/share/vm/prims/jvmtiExport.cpp Wed Dec 21 18:22:14 2011 -0800
+++ b/hotspot/src/share/vm/prims/jvmtiExport.cpp Thu Dec 22 12:50:42 2011 -0800
@@ -538,8 +538,6 @@
_curr_env = NULL;
_cached_length_ptr = cached_length_ptr;
_cached_data_ptr = cached_data_ptr;
- *_cached_length_ptr = 0;
- *_cached_data_ptr = NULL;
_state = _thread->jvmti_thread_state();
if (_state != NULL) {
--- a/hotspot/src/share/vm/prims/jvmtiRedefineClasses.cpp Wed Dec 21 18:22:14 2011 -0800
+++ b/hotspot/src/share/vm/prims/jvmtiRedefineClasses.cpp Thu Dec 22 12:50:42 2011 -0800
@@ -854,8 +854,9 @@
// RC_TRACE_WITH_THREAD macro has an embedded ResourceMark
RC_TRACE_WITH_THREAD(0x00000001, THREAD,
- ("loading name=%s (avail_mem=" UINT64_FORMAT "K)",
- the_class->external_name(), os::available_memory() >> 10));
+ ("loading name=%s kind=%d (avail_mem=" UINT64_FORMAT "K)",
+ the_class->external_name(), _class_load_kind,
+ os::available_memory() >> 10));
ClassFileStream st((u1*) _class_defs[i].class_bytes,
_class_defs[i].class_byte_count, (char *)"__VM_RedefineClasses__");
@@ -3205,8 +3206,20 @@
// with them was cached on the scratch class, move to the_class.
// Note: we still want to do this if nothing needed caching since it
// should get cleared in the_class too.
- the_class->set_cached_class_file(scratch_class->get_cached_class_file_bytes(),
- scratch_class->get_cached_class_file_len());
+ if (the_class->get_cached_class_file_bytes() == 0) {
+ // the_class doesn't have a cache yet so copy it
+ the_class->set_cached_class_file(
+ scratch_class->get_cached_class_file_bytes(),
+ scratch_class->get_cached_class_file_len());
+ }
+#ifndef PRODUCT
+ else {
+ assert(the_class->get_cached_class_file_bytes() ==
+ scratch_class->get_cached_class_file_bytes(), "cache ptrs must match");
+ assert(the_class->get_cached_class_file_len() ==
+ scratch_class->get_cached_class_file_len(), "cache lens must match");
+ }
+#endif
// Replace inner_classes
typeArrayOop old_inner_classes = the_class->inner_classes();