United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
Bug ID: JDK-7121600 Instrumentation.redefineClasses() leaks class bytes
JDK-7121600 : Instrumentation.redefineClasses() leaks class bytes

Details
Type:
Bug
Submit Date:
2011-12-14
Status:
Closed
Updated Date:
2014-01-15
Project Name:
JDK
Resolved Date:
2012-01-10
Component:
core-svc
OS:
generic
Sub-Component:
java.lang.instrument
CPU:
generic
Priority:
P2
Resolution:
Fixed
Affected Versions:
6
Fixed Versions:

Related Reports
Backport:
Backport:
Backport:
Relates:
Relates:
Relates:

Sub Tasks

Description
java.lang.instrument.Instrumentation.redefineClasses() leaks
class bytes for the redefined class or classes.

As part of the work on this bug, the following SDK/JDK test
will be added:

    java/lang/instrument/RedefineBigClass.sh
The attached RedefineBigClass.server.jtr.00 tells the tale
for the Server VM on Solaris X86. Version info is:

java version "1.6.0_29"
Java(TM) SE Runtime Environment (build 1.6.0_29-b11)
Java HotSpot(TM) Server VM (build 20.4-b02, mixed mode)


Here is trace info for the first RedefineClasses() call:

RedefineClasses-0x1: loading name=BigClass (avail_mem=323700K)
RedefineClasses-0x1: loaded name=BigClass (avail_mem=321536K)
RedefineClasses-0x1: redefined name=BigClass, count=1 (avail_mem=321504K)


Here is trace info for the last RedefineClasses() call:

RedefineClasses-0x1: loading name=BigClass (avail_mem=106516K)
RedefineClasses-0x1: loaded name=BigClass (avail_mem=106456K)
RedefineClasses-0x1: redefined name=BigClass, count=1000 (avail_mem=106456K)


Here's the math for leak:

321504 - 106456
215048
. / 1024
210.0078
. / 1024
.2050

Don't know why but the Server VM leaked way less than the Client VM
in this run.
The attached RedefineBigClass.client.jtr.00 tells the tale
for the Client VM on Solaris X86. Version info is:

java version "1.6.0_29"
Java(TM) SE Runtime Environment (build 1.6.0_29-b11)
Java HotSpot(TM) Client VM (build 20.4-b02, mixed mode)


Here is trace info for the first RedefineClasses() call:

RedefineClasses-0x1: loading name=BigClass (avail_mem=1185256K)
RedefineClasses-0x1: loaded name=BigClass (avail_mem=1184172K)
RedefineClasses-0x1: redefined name=BigClass, count=1 (avail_mem=1184140K)


Here is trace info for the last RedefineClasses() call:

RedefineClasses-0x1: loading name=BigClass (avail_mem=138252K)
RedefineClasses-0x1: loaded name=BigClass (avail_mem=138252K)
RedefineClasses-0x1: redefined name=BigClass, count=1000 (avail_mem=138252K)


Here's the math for leak:

1184140 - 138252
1045888
. / 1024
1021.3750
. / 1024
.9974

So we leaked almost 1GB in 1000 RedefineClasses() calls.
I did another Server VM run:

Here is trace info for the first RedefineClasses() call:

RedefineClasses-0x1: loading name=BigClass (avail_mem=1767152K)
RedefineClasses-0x1: loaded name=BigClass (avail_mem=1764988K)
RedefineClasses-0x1: redefined name=BigClass, count=1 (avail_mem=1764956K)


Here is trace info for the last RedefineClasses() call:

RedefineClasses-0x1: loading name=BigClass (avail_mem=640444K)
RedefineClasses-0x1: loaded name=BigClass (avail_mem=640388K)
RedefineClasses-0x1: redefined name=BigClass, count=1000 (avail_mem=640388K)


Here's the math:

1764956 - 640388
1124568
. / 1024
1098.2109
. / 1024
1.0724

So we leaked more than 1GB in 1000 RedefineClasses() calls.
Not sure at all what happened in the other Server run.

                                    

Comments
EVALUATION

Changeset: 3c1ab134db71
Author:    dcubed
Date:      2011-12-22 18:35 -0800
URL:       http://hg.openjdk.java.net/jdk8/tl/jdk/rev/3c1ab134db71

7121600: Instrumentation.redefineClasses() leaks class bytes
Summary: Call JNI ReleaseByteArrayElements() on memory returned by JNI GetByteArrayElements(). Also push test for 7122253.
Reviewed-by: acorn, poonam

! src/share/instrument/JPLISAgent.c
+ test/java/lang/instrument/BigClass.java
+ test/java/lang/instrument/MakeJAR4.sh
+ test/java/lang/instrument/RedefineBigClass.sh
+ test/java/lang/instrument/RedefineBigClassAgent.java
+ test/java/lang/instrument/RedefineBigClassApp.java
+ test/java/lang/instrument/RetransformBigClass.sh
+ test/java/lang/instrument/RetransformBigClassAgent.java
+ test/java/lang/instrument/RetransformBigClassApp.java


Note that the RetransformBigClass.sh test is for 7122253.
It is disabled in this push and should remain disabled
until the fix for 7122253 is in a promoted build.
                                     
2011-12-23
EVALUATION

The class bytes are extracted from their Java representation
via a JNI GetByteArrayElements() call. That method can make
a copy of the data and the copy must be released via a JNI
ReleaseByteArrayElements().
                                     
2011-12-14
SUGGESTED FIX

The proposed fix for code review round 0 is attached as 7121600-webrev-cr0.tgz.

Update: The proposed fix for code review round 0 solves the memory
leak bug but not properly handle error conditions.
                                     
2011-12-14
PUBLIC COMMENTS

I went back to the JDK1.5.0 FCS/GA source coode and found that
the j2se/src/share/instrument/JPLISAgent.c source for the

    redefineClasses(JNIEnv * jnienv, JPLISAgent * agent,
        jobjectArray classDefinitions)

is unchanged since revision 1.1:

D 1.1 03/08/16 10:43:00 rfield 1 0      01256/00000/00000
MRs:
COMMENTS:
date and time created 03/08/16 10:43:00 by rfield

Here's the code in 'sccs get -p -m' format:

1.1     /*
1.1      *  Java code must not call this with a null list or a zero-length list.
1.1      */
1.1     void
1.1     redefineClasses(JNIEnv * jnienv, JPLISAgent * agent, jobjectArray classDefinitions) {
1.1         jvmtiEnv*   jvmtienv                        = agent->mJVMTIEnv;
1.1         jboolean    errorOccurred                   = JNI_FALSE;
1.1         jclass      classDefClass                   = NULL;
1.1         jmethodID   getDefinitionClassMethodID      = NULL;
1.1         jmethodID   getDefinitionClassFileMethodID  = NULL;
1.1         jvmtiClassDefinition* classDefs             = NULL;
1.1         jsize       numDefs                         = 0;
1.1
1.1         jplis_assert(classDefinitions != NULL);
1.1
1.1         numDefs = (*jnienv)->GetArrayLength(jnienv, classDefinitions);
1.1         errorOccurred = checkForThrowable(jnienv);
1.1         jplis_assert(!errorOccurred);
1.1
1.1         if (!errorOccurred) {
1.1             jplis_assert(numDefs > 0);
1.1             /* get method IDs for methods to call on class definitions */
1.1             classDefClass = (*jnienv)->FindClass(jnienv, "java/lang/instrument/ClassDefinition");
1.1             errorOccurred = checkForThrowable(jnienv);
1.1             jplis_assert(!errorOccurred);
1.1         }
1.1
1.1         if (!errorOccurred) {
1.1             getDefinitionClassMethodID = (*jnienv)->GetMethodID(    jnienv,
1.1                                                     classDefClass,
1.1                                                     "getDefinitionClass",
1.1                                                     "()Ljava/lang/Class;");
1.1             errorOccurred = checkForThrowable(jnienv);
1.1             jplis_assert(!errorOccurred);
1.1         }
1.1
1.1         if (!errorOccurred) {
1.1             getDefinitionClassFileMethodID = (*jnienv)->GetMethodID(    jnienv,
1.1                                                         classDefClass,
1.1                                                         "getDefinitionClassFile",
1.1                                                         "()[B");
1.1             errorOccurred = checkForThrowable(jnienv);
1.1             jplis_assert(!errorOccurred);
1.1         }
1.1
1.1         if (!errorOccurred) {
1.1             classDefs = (jvmtiClassDefinition *) allocate(
1.1                                                     jvmtienv,
1.1                                                     numDefs * sizeof(jvmtiClassDefinition));
1.1             errorOccurred = (classDefs == NULL);
1.1             jplis_assert(!errorOccurred);
1.1             if ( errorOccurred ) {
1.1                 createAndThrowThrowableFromJVMTIErrorCode(jnienv, JVMTI_ERROR_OUT_OF_MEMORY);
1.1             }
1.1             else {
1.1                 jint i;
1.1                 for (i = 0; i < numDefs; i++) {
1.1                     jclass      classDef    = NULL;
1.1                     jbyteArray  targetFile  = NULL;
1.1
1.1                     classDef = (*jnienv)->GetObjectArrayElement(jnienv, classDefinitions, i);
1.1                     errorOccurred = checkForThrowable(jnienv);
1.1                     jplis_assert(!errorOccurred);
1.1                     if (errorOccurred) {
1.1                         break;
1.1                     }
1.1
1.1                     classDefs[i].klass = (*jnienv)->CallObjectMethod(jnienv, classDef, getDefinitionClassMethodID);
1.1                     errorOccurred = checkForThrowable(jnienv);
1.1                     jplis_assert(!errorOccurred);
1.1                     if (errorOccurred) {
1.1                         break;
1.1                     }
1.1
1.1                     targetFile = (*jnienv)->CallObjectMethod(jnienv, classDef, getDefinitionClassFileMethodID);
1.1                     errorOccurred = checkForThrowable(jnienv);
1.1                     jplis_assert(!errorOccurred);
1.1                     if (errorOccurred) {
1.1                         break;
1.1                     }
1.1
1.1                     classDefs[i].class_bytes = (unsigned char*)(*jnienv)->GetByteArrayElements(jnienv, targetFile, NULL);
1.1                     errorOccurred = checkForThrowable(jnienv);
1.1                     jplis_assert(!errorOccurred);
1.1                     if (errorOccurred) {
1.1                         break;
1.1                     }
1.1
1.1                     classDefs[i].class_byte_count = (*jnienv)->GetArrayLength(jnienv, targetFile);
1.1                     errorOccurred = checkForThrowable(jnienv);
1.1                     jplis_assert(!errorOccurred);
1.1                     if (errorOccurred) {
1.1                         break;
1.1                     }
1.1                 }
1.1
1.1                 if (!errorOccurred) {
1.1                     jvmtiError  errorCode = JVMTI_ERROR_NONE;
1.1                     errorCode = (*jvmtienv)->RedefineClasses(jvmtienv, numDefs, classDefs);
1.1                     errorOccurred = (errorCode != JVMTI_ERROR_NONE);
1.1                     if ( errorOccurred ) {
1.1                         createAndThrowThrowableFromJVMTIErrorCode(jnienv, errorCode);
1.1                     }
1.1                 }
1.1
1.1                 /* Give back the buffer if we allocated it.
1.1                  */
1.1                 deallocate(jvmtienv, classDefs);
1.1             }
1.1         }
1.1
1.1         mapThrownThrowableIfNecessary(jnienv, redefineClassMapper);
1.1     }


I don't know which JDK5 build included the JLI/JPLIS push
so I'm using 5.0-B64 which is the FCS/GA build.
                                     
2011-12-20
SUGGESTED FIX

The proposed fix for code review round 1 is attached as
7121600-webrev-cr1.tgz. This archive contains webrevs
for JDK6u29, JDK7u4 and JDK8.
                                     
2011-12-22
SUGGESTED FIX

Minor changes made during Code Review round 1:

$ diff -c src/share/instrument/JPLISAgent.c{.cr1,}
*** src/share/instrument/JPLISAgent.c.cr1       Tue Dec 20 09:39:38 2011
--- src/share/instrument/JPLISAgent.c   Wed Dec 21 14:40:34 2011
***************
*** 1209,1215 ****
              createAndThrowThrowableFromJVMTIErrorCode(jnienv, JVMTI_ERROR_OUT_OF_MEMORY);
          }
  
!         if (!errorOccurred) {
              /*
               * We have to save the targetFile values that we compute so
               * that we can release the class_bytes arrays that are
--- 1209,1215 ----
              createAndThrowThrowableFromJVMTIErrorCode(jnienv, JVMTI_ERROR_OUT_OF_MEMORY);
          }
  
!         else {
              /*
               * We have to save the targetFile values that we compute so
               * that we can release the class_bytes arrays that are
***************
*** 1308,1316 ****
                          if (!errorOccurred) {
                              errorOccurred = checkForThrowable(jnienv);
                              jplis_assert(!errorOccurred);
-                             if (errorOccurred) {
-                                 break;
-                             }
                          }
                      }
                  }
--- 1308,1313 ----

$ diff -c test/java/lang/instrument/RetransformBigClass.sh{.cr1,}
*** test/java/lang/instrument/RetransformBigClass.sh.cr1        Tue Dec 20 15:35:49 2011
--- test/java/lang/instrument/RetransformBigClass.sh    Thu Dec 22 18:25:35 2011
***************
*** 23,28 ****
--- 23,29 ----
  
  # @test
  # @bug 7122253
+ # @ignore until the fix for 7122253 (from HotSpot) is in a promoted build
  # @summary Retransform a big class.
  # @author Daniel D. Daugherty
  #
                                     
2011-12-23



Hardware and Software, Engineered to Work Together