EVALUATION

http://hg.openjdk.java.net/lambda/lambda/hotspot/rev/4ceaf61479fc
                                     

EVALUATION

http://hg.openjdk.java.net/hsx/hotspot-main/hotspot/rev/4ceaf61479fc
                                     

SUGGESTED FIX

The proposed fix for code review round 0 is attached as
7122253-webrev-cr0.tgz. This archive contains webrevs
for JDK6u29/HSX-20, JDK7u4/HSX-23-B06 and JDK8/HSX-23-B08.
                                     

EVALUATION

http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/4ceaf61479fc
                                     

PUBLIC COMMENTS

JVM/TI RetransformClasses() class file byte caching was introduced
in JDK6-B40. Here are the relevant code snippets:

src/share/vm/prims/jvmtiRedefineClasses.cpp:

1.17      transfer_old_native_function_registrations();
1.1
1.37      // The class file bytes from before any retransformable agents mucked
1.37      // with them was cached on the scratch class, move to the_class.
1.37      // Note: we still want to do this if nothing needed caching since it
1.37      // should get cleared in the_class too.
1.37      the_class->set_cached_class_file(scratch_class->get_cached_class_file_bytes(),
1.37                                       scratch_class->get_cached_class_file_len());
1.37


The above set_cached_class_file() call is where the
previous cached value (if any) gets lost. Here is the
delta that made the above changes:

D 1.37 05/05/20 22:56:19 rfield 83 82   00013/00004/01858
MRs:
COMMENTS:
4772582: JVM TI - multiple independent simultaneous instrumenting agents - RetransformClasses
              Add class file caching; Load kind


src/share/vm/runtime/classFileParser.cpp:

1.248.1.2         // So that JVMTI can cache class file in the state before retransformable agents
1.248.1.2         // have modified it
1.248.1.1         unsigned char *cached_class_file_bytes = NULL;
1.248.1.1         jint cached_class_file_length;
1.248.1.1
1.242     ClassFileStream* cfs = stream();

<snip>

1.190     if (JvmtiExport::should_post_class_file_load_hook()) {
1.187.1.1           unsigned char* ptr = cfs->buffer();
1.192       unsigned char* end_ptr = cfs->buffer() + cfs->length();
1.138.3.1
1.248.1.1           JvmtiExport::post_class_file_load_hook(name, class_loader, protection_domain,
1.248.1.1                                                  &ptr, &end_ptr,
1.248.1.1                                                  &cached_class_file_bytes,
1.248.1.1                                                  &cached_class_file_length);


The above snippets of code are where the cached class file bytes
are anchored in order to get filled in by the ClassFileLoadHook
event handler. cached_class_file_bytes is initialized to zero
here instead of being looked up on 'the_class'. Here's the delta
that made the above changes:

D 1.248.1.1 05/05/20 22:59:45 rfield 450 449    00014/00001/03713
MRs:
COMMENTS:
4772582: JVM TI - multiple independent simultaneous instrumenting agents - RetransformClasses
              Add class file caching


src/share/vm/prims/jvmtiExport.cpp

1.102       _cached_data_ptr = cached_data_ptr;
1.102       *_cached_length_ptr = 0;
1.102       *_cached_data_ptr = NULL;


The above snippet of code is where the cache class file
bytes would get lost even if we had copied the pointer
from 'the_class'. Here is the delta that made the above
changes:

D 1.102 05/05/20 22:52:44 rfield 181 180        00159/00069/02319
MRs:
COMMENTS:
4772582: JVM TI - multiple independent simultaneous instrumenting agents - RetransformClasses
              Create CFLH poster class to: add class file caching, and use new CFLH order


So this memory leak has been around since JDK6-B40.
                                     

EVALUATION

The instanceKlass:_cached_class_file_bytes field is
getting over written so we're leaking a malloc()'ed
array the size of the class bytes.

This leak happens on the second and subsequent calls
to JVM/TI RetransformClasses().
                                     

EVALUATION

ClassFileParser::parseClassFile() is the anchor point
for the caches during class file parsing. It should use
the "class_being_redefined" cache on the JvmtiThreadState
to find the cache values for "the_class" so they can be
used during the parsing of "scratch_class".