From 3be4899371784c98d175393f71d416c3ca4d6990 Mon Sep 17 00:00:00 2001 From: dcubed Date: Thu, 22 Dec 2011 12:50:42 -0800 Subject: [PATCH] 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 --- src/share/vm/classfile/classFileParser.cpp | 22 +++++++++++++++++++-- src/share/vm/prims/jvmtiEnv.cpp | 5 ++++- src/share/vm/prims/jvmtiExport.cpp | 2 -- src/share/vm/prims/jvmtiRedefineClasses.cpp | 21 ++++++++++++++++---- 4 files changed, 41 insertions(+), 9 deletions(-) diff --git a/src/share/vm/classfile/classFileParser.cpp b/src/share/vm/classfile/classFileParser.cpp index b28b8ad7e..f018657e9 100644 --- a/src/share/vm/classfile/classFileParser.cpp +++ b/src/share/vm/classfile/classFileParser.cpp @@ -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 @@ instanceKlassHandle ClassFileParser::parseClassFile(Symbol* name, 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 @@ instanceKlassHandle ClassFileParser::parseClassFile(Symbol* name, _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(); diff --git a/src/share/vm/prims/jvmtiEnv.cpp b/src/share/vm/prims/jvmtiEnv.cpp index 608491501..46519bb57 100644 --- a/src/share/vm/prims/jvmtiEnv.cpp +++ b/src/share/vm/prims/jvmtiEnv.cpp @@ -267,7 +267,10 @@ JvmtiEnv::RetransformClasses(jint class_count, const jclass* classes) { 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 diff --git a/src/share/vm/prims/jvmtiExport.cpp b/src/share/vm/prims/jvmtiExport.cpp index 181716d4c..5325073b6 100644 --- a/src/share/vm/prims/jvmtiExport.cpp +++ b/src/share/vm/prims/jvmtiExport.cpp @@ -538,8 +538,6 @@ class JvmtiClassFileLoadHookPoster : public StackObj { _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) { diff --git a/src/share/vm/prims/jvmtiRedefineClasses.cpp b/src/share/vm/prims/jvmtiRedefineClasses.cpp index 10a478bd5..295ed8664 100644 --- a/src/share/vm/prims/jvmtiRedefineClasses.cpp +++ b/src/share/vm/prims/jvmtiRedefineClasses.cpp @@ -854,8 +854,9 @@ jvmtiError VM_RedefineClasses::load_new_class_versions(TRAPS) { // 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 @@ void VM_RedefineClasses::redefine_single_class(jclass the_jclass, // 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(); -- GitLab