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

jdk/src/share/instrument/JPLISAgent.c

@@ -1164,6 +1164,7 @@ redefineClasses(JNIEnv * jnienv, JPLISAgent * agent, jobjectArray classDefinitio
     jmethodID   getDefinitionClassMethodID      = NULL;
     jmethodID   getDefinitionClassFileMethodID  = NULL;
     jvmtiClassDefinition* classDefs             = NULL;
+    jbyteArray* targetFiles                     = NULL;
     jsize       numDefs                         = 0;
 
     jplis_assert(classDefinitions != NULL);
@@ -1207,61 +1208,112 @@ redefineClasses(JNIEnv * jnienv, JPLISAgent * agent, jobjectArray classDefinitio
         if ( errorOccurred ) {
             createAndThrowThrowableFromJVMTIErrorCode(jnienv, JVMTI_ERROR_OUT_OF_MEMORY);
         }
+
         else {
-            jint i;
-            for (i = 0; i < numDefs; i++) {
-                jclass      classDef    = NULL;
-                jbyteArray  targetFile  = NULL;
+            /*
+             * We have to save the targetFile values that we compute so
+             * that we can release the class_bytes arrays that are
+             * returned by GetByteArrayElements(). In case of a JNI
+             * error, we can't (easily) recompute the targetFile values
+             * and we still want to free any memory we allocated.
+             */
+            targetFiles = (jbyteArray *) allocate(jvmtienv,
+                                                  numDefs * sizeof(jbyteArray));
+            errorOccurred = (targetFiles == NULL);
+            jplis_assert(!errorOccurred);
+            if ( errorOccurred ) {
+                deallocate(jvmtienv, (void*)classDefs);
+                createAndThrowThrowableFromJVMTIErrorCode(jnienv,
+                    JVMTI_ERROR_OUT_OF_MEMORY);
+            }
+            else {
+                jint i, j;
 
-                classDef = (*jnienv)->GetObjectArrayElement(jnienv, classDefinitions, i);
-                errorOccurred = checkForThrowable(jnienv);
-                jplis_assert(!errorOccurred);
-                if (errorOccurred) {
-                    break;
-                }
+                // clear classDefs so we can correctly free memory during errors
+                memset(classDefs, 0, numDefs * sizeof(jvmtiClassDefinition));
 
-                classDefs[i].klass = (*jnienv)->CallObjectMethod(jnienv, classDef, getDefinitionClassMethodID);
-                errorOccurred = checkForThrowable(jnienv);
-                jplis_assert(!errorOccurred);
-                if (errorOccurred) {
-                    break;
-                }
+                for (i = 0; i < numDefs; i++) {
+                    jclass      classDef    = NULL;
 
-                targetFile = (*jnienv)->CallObjectMethod(jnienv, classDef, getDefinitionClassFileMethodID);
-                errorOccurred = checkForThrowable(jnienv);
-                jplis_assert(!errorOccurred);
-                if (errorOccurred) {
-                    break;
-                }
+                    classDef = (*jnienv)->GetObjectArrayElement(jnienv, classDefinitions, i);
+                    errorOccurred = checkForThrowable(jnienv);
+                    jplis_assert(!errorOccurred);
+                    if (errorOccurred) {
+                        break;
+                    }
 
-                classDefs[i].class_bytes = (unsigned char*)(*jnienv)->GetByteArrayElements(jnienv, targetFile, NULL);
-                errorOccurred = checkForThrowable(jnienv);
-                jplis_assert(!errorOccurred);
-                if (errorOccurred) {
-                    break;
+                    classDefs[i].klass = (*jnienv)->CallObjectMethod(jnienv, classDef, getDefinitionClassMethodID);
+                    errorOccurred = checkForThrowable(jnienv);
+                    jplis_assert(!errorOccurred);
+                    if (errorOccurred) {
+                        break;
+                    }
+
+                    targetFiles[i] = (*jnienv)->CallObjectMethod(jnienv, classDef, getDefinitionClassFileMethodID);
+                    errorOccurred = checkForThrowable(jnienv);
+                    jplis_assert(!errorOccurred);
+                    if (errorOccurred) {
+                        break;
+                    }
+
+                    classDefs[i].class_byte_count = (*jnienv)->GetArrayLength(jnienv, targetFiles[i]);
+                    errorOccurred = checkForThrowable(jnienv);
+                    jplis_assert(!errorOccurred);
+                    if (errorOccurred) {
+                        break;
+                    }
+
+                    /*
+                     * Allocate class_bytes last so we don't have to free
+                     * memory on a partial row error.
+                     */
+                    classDefs[i].class_bytes = (unsigned char*)(*jnienv)->GetByteArrayElements(jnienv, targetFiles[i], NULL);
+                    errorOccurred = checkForThrowable(jnienv);
+                    jplis_assert(!errorOccurred);
+                    if (errorOccurred) {
+                        break;
+                    }
                 }
 
-                classDefs[i].class_byte_count = (*jnienv)->GetArrayLength(jnienv, targetFile);
-                errorOccurred = checkForThrowable(jnienv);
-                jplis_assert(!errorOccurred);
-                if (errorOccurred) {
-                    break;
+                if (!errorOccurred) {
+                    jvmtiError  errorCode = JVMTI_ERROR_NONE;
+                    errorCode = (*jvmtienv)->RedefineClasses(jvmtienv, numDefs, classDefs);
+                    if (errorCode == JVMTI_ERROR_WRONG_PHASE) {
+                        /* insulate caller from the wrong phase error */
+                        errorCode = JVMTI_ERROR_NONE;
+                    } else {
+                        errorOccurred = (errorCode != JVMTI_ERROR_NONE);
+                        if ( errorOccurred ) {
+                            createAndThrowThrowableFromJVMTIErrorCode(jnienv, errorCode);
+                        }
+                    }
                 }
-            }
 
-            if (!errorOccurred) {
-                jvmtiError  errorCode = JVMTI_ERROR_NONE;
-                errorCode = (*jvmtienv)->RedefineClasses(jvmtienv, numDefs, classDefs);
-                check_phase_blob_ret(errorCode, deallocate(jvmtienv, (void*)classDefs));
-                errorOccurred = (errorCode != JVMTI_ERROR_NONE);
-                if ( errorOccurred ) {
-                    createAndThrowThrowableFromJVMTIErrorCode(jnienv, errorCode);
+                /*
+                 * Cleanup memory that we allocated above. If we had a
+                 * JNI error, a JVM/TI error or no errors, index 'i'
+                 * tracks how far we got in processing the classDefs
+                 * array. Note:  ReleaseByteArrayElements() is safe to
+                 * call with a JNI exception pending.
+                 */
+                for (j = 0; j < i; j++) {
+                    if ((jbyte *)classDefs[j].class_bytes != NULL) {
+                        (*jnienv)->ReleaseByteArrayElements(jnienv,
+                            targetFiles[j], (jbyte *)classDefs[j].class_bytes,
+                            0 /* copy back and free */);
+                        /*
+                         * Only check for error if we didn't already have one
+                         * so we don't overwrite errorOccurred.
+                         */
+                        if (!errorOccurred) {
+                            errorOccurred = checkForThrowable(jnienv);
+                            jplis_assert(!errorOccurred);
+                        }
+                    }
                 }
+                deallocate(jvmtienv, (void*)targetFiles);
+                deallocate(jvmtienv, (void*)classDefs);
             }
-
-            /* Give back the buffer if we allocated it.
-             */
-            deallocate(jvmtienv, (void*)classDefs);
         }
     }
 

jdk/test/java/lang/instrument/MakeJAR4.sh

+#!/bin/sh
+AGENT="$1"
+OTHER="$2"
+shift 2
+
+if [ "${TESTSRC}" = "" ]
+then
+  echo "TESTSRC not set.  Test cannot execute.  Failed."
+  exit 1
+fi
+echo "TESTSRC=${TESTSRC}"
+
+if [ "${TESTJAVA}" = "" ]
+then
+  echo "TESTJAVA not set.  Test cannot execute.  Failed."
+  exit 1
+fi
+echo "TESTJAVA=${TESTJAVA}"
+
+if [ "${TESTCLASSES}" = "" ]
+then
+  echo "TESTCLASSES not set.  Test cannot execute.  Failed."
+  exit 1
+fi
+
+JAVAC="${TESTJAVA}/bin/javac -g"
+JAR="${TESTJAVA}/bin/jar"
+
+cp ${TESTSRC}/${AGENT}.java ${TESTSRC}/${OTHER}.java .
+${JAVAC} ${AGENT}.java ${OTHER}.java
+
+echo "Manifest-Version: 1.0"    >  ${AGENT}.mf
+echo Premain-Class: ${AGENT} >> ${AGENT}.mf
+while [ $# != 0 ] ; do
+  echo $1 >> ${AGENT}.mf
+  shift
+done
+
+
+${JAR} cvfm ${AGENT}.jar ${AGENT}.mf ${AGENT}*.class ${OTHER}*.java

jdk/test/java/lang/instrument/RedefineBigClass.sh

+#
+# Copyright (c) 2011, Oracle and/or its affiliates. All rights reserved.
+# DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+#
+# This code is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License version 2 only, as
+# published by the Free Software Foundation.
+#
+# This code is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+# FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+# version 2 for more details (a copy is included in the LICENSE file that
+# accompanied this code).
+#
+# You should have received a copy of the GNU General Public License version
+# 2 along with this work; if not, write to the Free Software Foundation,
+# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+#
+# Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+# or visit www.oracle.com if you need additional information or have any
+# questions.
+#
+
+# @test
+# @bug 7121600
+# @summary Redefine a big class.
+# @author Daniel D. Daugherty
+#
+# @run shell MakeJAR3.sh RedefineBigClassAgent 'Can-Redefine-Classes: true'
+# @run build BigClass RedefineBigClassApp
+# @run shell/timeout=600 RedefineBigClass.sh
+#
+
+if [ "${TESTJAVA}" = "" ]
+then
+  echo "TESTJAVA not set.  Test cannot execute.  Failed."
+  exit 1
+fi
+
+if [ "${TESTSRC}" = "" ]
+then
+  echo "TESTSRC not set.  Test cannot execute.  Failed."
+  exit 1
+fi
+
+if [ "${TESTCLASSES}" = "" ]
+then
+  echo "TESTCLASSES not set.  Test cannot execute.  Failed."
+  exit 1
+fi
+
+JAVAC="${TESTJAVA}"/bin/javac
+JAVA="${TESTJAVA}"/bin/java
+
+"${JAVA}" ${TESTVMOPTS} \
+    -XX:TraceRedefineClasses=3 \
+    -javaagent:RedefineBigClassAgent.jar=BigClass.class \
+    -classpath "${TESTCLASSES}" RedefineBigClassApp \
+    > output.log 2>&1
+result=$?
+
+cat output.log
+
+if [ "$result" = 0 ]; then
+    echo "PASS: RedefineBigClassApp exited with status of 0."
+else
+    echo "FAIL: RedefineBigClassApp exited with status of $result"
+    exit "$result"
+fi
+
+MESG="Exception"
+grep "$MESG" output.log
+result=$?
+if [ "$result" = 0 ]; then
+    echo "FAIL: found '$MESG' in the test output"
+    result=1
+else
+    echo "PASS: did NOT find '$MESG' in the test output"
+    result=0
+fi
+
+exit $result

jdk/test/java/lang/instrument/RedefineBigClassAgent.java

+/*
+ * Copyright (c) 2011, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+import java.lang.instrument.*;
+import java.net.*;
+import java.util.*;
+import java.io.*;
+
+public class RedefineBigClassAgent {
+    // N_REDEFINES = 2000 made my Solaris X86 box crawl on its knees
+    // with the Server VM. The timeout is 600 seconds, but the test
+    // ran for 675 seconds so it took the harness 75 seconds to kill
+    // the test.
+    private static int N_REDEFINES = 1000;
+    public static Class clz;
+    public static volatile boolean doneRedefining = false;
+
+    // just read the original class and redefine in a loop via a Timer
+    public static void premain(String agentArgs, final Instrumentation inst) throws Exception {
+        String s = agentArgs.substring(0, agentArgs.indexOf(".class"));
+        clz = Class.forName(s.replace('/', '.'));
+        ClassLoader loader =
+            RedefineBigClassAgent.class.getClassLoader();
+        URL classURL = loader.getResource(agentArgs);
+        if (classURL == null) {
+            throw new Exception("Cannot find class: " + agentArgs);
+        }
+
+        int         redefineLength;
+        InputStream redefineStream;
+
+        System.out.println("Reading test class from " + classURL);
+        if (classURL.getProtocol().equals("file")) {
+            File f = new File(classURL.getFile());
+            redefineStream = new FileInputStream(f);
+            redefineLength = (int) f.length();
+        } else {
+            URLConnection conn = classURL.openConnection();
+            redefineStream = conn.getInputStream();
+            redefineLength = conn.getContentLength();
+        }
+
+        final byte[] buffer = new byte[redefineLength];
+        new BufferedInputStream(redefineStream).read(buffer);
+
+        System.gc();  // throw away anything we can before we start testing
+
+        new Timer(true).schedule(new TimerTask() {
+            public void run() {
+                try {
+                    int i;
+                    System.out.println("Redefining");
+                    ClassDefinition cld = new ClassDefinition(clz, buffer);
+                    for (i = 0; i < N_REDEFINES; i++) {
+                        inst.redefineClasses(new ClassDefinition[] { cld });
+                        System.gc();  // throw away anything we can
+                    }
+                    System.out.println("Redefined " + i + " times.");
+                    RedefineBigClassAgent.doneRedefining = true;
+                }
+                catch (Exception e) { e.printStackTrace(); }
+            }
+        }, 500);
+    }
+}

jdk/test/java/lang/instrument/RedefineBigClassApp.java

+/*
+ * Copyright (c) 2011, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+public class RedefineBigClassApp {
+    public static void main(String[] args) throws Exception {
+        System.out.println("Creating instance of " +
+            RedefineBigClassAgent.clz);
+        RedefineBigClassAgent.clz.newInstance();
+
+        int count = 0;
+        while (!RedefineBigClassAgent.doneRedefining) {
+            System.out.println("App loop count: " + ++count);
+            try {
+                Thread.sleep(10 * 1000);
+            } catch (InterruptedException ie) {
+            }
+        }
+        System.out.println("App looped  " + count + " times.");
+
+        System.exit(0);
+    }
+}

jdk/test/java/lang/instrument/RetransformBigClass.sh

+#
+# Copyright (c) 2011, Oracle and/or its affiliates. All rights reserved.
+# DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+#
+# This code is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License version 2 only, as
+# published by the Free Software Foundation.
+#
+# This code is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+# FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+# version 2 for more details (a copy is included in the LICENSE file that
+# accompanied this code).
+#
+# You should have received a copy of the GNU General Public License version
+# 2 along with this work; if not, write to the Free Software Foundation,
+# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+#
+# Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+# or visit www.oracle.com if you need additional information or have any
+# questions.
+#
+
+# @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
+#
+# @run shell MakeJAR4.sh RetransformBigClassAgent SimpleIdentityTransformer 'Can-Retransform-Classes: true'
+# @run build BigClass RetransformBigClassApp
+# @run shell/timeout=600 RetransformBigClass.sh
+#
+
+if [ "${TESTJAVA}" = "" ]
+then
+  echo "TESTJAVA not set.  Test cannot execute.  Failed."
+  exit 1
+fi
+
+if [ "${TESTSRC}" = "" ]
+then
+  echo "TESTSRC not set.  Test cannot execute.  Failed."
+  exit 1
+fi
+
+if [ "${TESTCLASSES}" = "" ]
+then
+  echo "TESTCLASSES not set.  Test cannot execute.  Failed."
+  exit 1
+fi
+
+JAVAC="${TESTJAVA}"/bin/javac
+JAVA="${TESTJAVA}"/bin/java
+
+"${JAVA}" ${TESTVMOPTS} \
+    -XX:TraceRedefineClasses=3 \
+    -javaagent:RetransformBigClassAgent.jar=BigClass.class \
+    -classpath "${TESTCLASSES}" RetransformBigClassApp \
+    > output.log 2>&1
+result=$?
+
+cat output.log
+
+if [ "$result" = 0 ]; then
+    echo "PASS: RetransformBigClassApp exited with status of 0."
+else
+    echo "FAIL: RetransformBigClassApp exited with status of $result"
+    exit "$result"
+fi
+
+MESG="Exception"
+grep "$MESG" output.log
+result=$?
+if [ "$result" = 0 ]; then
+    echo "FAIL: found '$MESG' in the test output"
+    result=1
+else
+    echo "PASS: did NOT find '$MESG' in the test output"
+    result=0
+fi
+
+exit $result

jdk/test/java/lang/instrument/RetransformBigClassAgent.java

+/*
+ * Copyright (c) 2011, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+import java.lang.instrument.*;
+import java.util.*;
+
+public class RetransformBigClassAgent {
+    private static int N_RETRANSFORMS = 1000;
+    public static Class clz;
+    public static volatile boolean doneRetransforming = false;
+
+    // just retransform the class in a loop via a Timer
+    public static void premain(String agentArgs, final Instrumentation inst) throws Exception {
+        String s = agentArgs.substring(0, agentArgs.indexOf(".class"));
+        clz = Class.forName(s.replace('/', '.'));
+
+        ClassFileTransformer trans = new SimpleIdentityTransformer();
+        inst.addTransformer(trans, true /* canRetransform */);
+
+        System.gc();  // throw away anything we can before we start testing
+
+        new Timer(true).schedule(new TimerTask() {
+            public void run() {
+                try {
+                    int i;
+                    System.out.println("Retransforming");
+                    for (i = 0; i < N_RETRANSFORMS; i++) {
+                        inst.retransformClasses(clz);
+                        System.gc();  // throw away anything we can
+                    }
+                    System.out.println("Retransformed " + i + " times.");
+                    RetransformBigClassAgent.doneRetransforming = true;
+                }
+                catch (Exception e) { e.printStackTrace(); }
+            }
+        }, 500);
+    }
+}

jdk/test/java/lang/instrument/RetransformBigClassApp.java

+/*
+ * Copyright (c) 2011, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+public class RetransformBigClassApp {
+    public static void main(String[] args) throws Exception {
+        System.out.println("Creating instance of " +
+            RetransformBigClassAgent.clz);
+        RetransformBigClassAgent.clz.newInstance();
+
+        int count = 0;
+        while (!RetransformBigClassAgent.doneRetransforming) {
+            System.out.println("App loop count: " + ++count);
+            try {
+                Thread.sleep(10 * 1000);
+            } catch (InterruptedException ie) {
+            }
+        }
+        System.out.println("App looped  " + count + " times.");
+
+        System.exit(0);
+    }
+}