/hg/icedtea8-forest/jdk: 8153711, PR3313: [REDO] JDWP: Memory Le...

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

/hg/icedtea8-forest/jdk: 8153711, PR3313: [REDO] JDWP: Memory Le...

andrew-127
changeset 9f6a0864a734 in /hg/icedtea8-forest/jdk
details: http://icedtea.classpath.org/hg/icedtea8-forest/jdk?cmd=changeset;node=9f6a0864a734
author: sgehwolf
date: Mon Mar 21 11:24:09 2016 +0100

        8153711, PR3313: [REDO] JDWP: Memory Leak: GlobalRefs never deleted when processing invokeMethod command
        Summary: Delete global references in invoker_completeInvokeRequest()
        Reviewed-by: sspitsyn, dsamersoff


diffstat:

 src/share/back/invoker.c                  |   94 ++++++-
 test/com/sun/jdi/oom/@debuggeeVMOptions   |    1 +
 test/com/sun/jdi/oom/OomDebugTest.java    |  417 ++++++++++++++++++++++++++++++
 test/com/sun/jdi/oom/OomDebugTestSetup.sh |   46 +++
 4 files changed, 555 insertions(+), 3 deletions(-)

diffs (truncated from 656 to 500 lines):

diff -r 1179be40f1e3 -r 9f6a0864a734 src/share/back/invoker.c
--- a/src/share/back/invoker.c Wed Jan 25 07:14:11 2017 +0000
+++ b/src/share/back/invoker.c Mon Mar 21 11:24:09 2016 +0100
@@ -211,6 +211,62 @@
     return error;
 }
 
+/*
+ * Delete saved global references - if any - for:
+ * - a potentially thrown Exception
+ * - a returned refernce/array value
+ * See invoker_doInvoke() and invoke* methods where global references
+ * are being saved.
+ */
+static void
+deletePotentiallySavedGlobalRefs(JNIEnv *env, InvokeRequest *request)
+{
+    /* Delete potentially saved return value */
+    if ((request->invokeType == INVOKE_CONSTRUCTOR) ||
+        (returnTypeTag(request->methodSignature) == JDWP_TAG(OBJECT)) ||
+        (returnTypeTag(request->methodSignature) == JDWP_TAG(ARRAY))) {
+        if (request->returnValue.l != NULL) {
+            tossGlobalRef(env, &(request->returnValue.l));
+        }
+    }
+    /* Delete potentially saved exception */
+    if (request->exception != NULL) {
+        tossGlobalRef(env, &(request->exception));
+    }
+}
+
+/*
+ * Delete global argument references from the request which got put there before a
+ * invoke request was carried out. See fillInvokeRequest().
+ */
+static void
+deleteGlobalArgumentRefs(JNIEnv *env, InvokeRequest *request)
+{
+    void *cursor;
+    jint argIndex = 0;
+    jvalue *argument = request->arguments;
+    jbyte argumentTag = firstArgumentTypeTag(request->methodSignature, &cursor);
+
+    if (request->clazz != NULL) {
+        tossGlobalRef(env, &(request->clazz));
+    }
+    if (request->instance != NULL) {
+        tossGlobalRef(env, &(request->instance));
+    }
+    /* Delete global argument references */
+    while (argIndex < request->argumentCount) {
+        if ((argumentTag == JDWP_TAG(OBJECT)) ||
+            (argumentTag == JDWP_TAG(ARRAY))) {
+            if (argument->l != NULL) {
+                tossGlobalRef(env, &(argument->l));
+            }
+        }
+        argument++;
+        argIndex++;
+        argumentTag = nextArgumentTypeTag(&cursor);
+    }
+}
+
 static jvmtiError
 fillInvokeRequest(JNIEnv *env, InvokeRequest *request,
                   jbyte invokeType, jbyte options, jint id,
@@ -320,6 +376,8 @@
 invokeConstructor(JNIEnv *env, InvokeRequest *request)
 {
     jobject object;
+
+    JDI_ASSERT_MSG(request->clazz, "Request clazz null");
     object = JNI_FUNC_PTR(env,NewObjectA)(env, request->clazz,
                                      request->method,
                                      request->arguments);
@@ -336,6 +394,7 @@
         case JDWP_TAG(OBJECT):
         case JDWP_TAG(ARRAY): {
             jobject object;
+            JDI_ASSERT_MSG(request->clazz, "Request clazz null");
             object = JNI_FUNC_PTR(env,CallStaticObjectMethodA)(env,
                                        request->clazz,
                                        request->method,
@@ -424,6 +483,7 @@
         case JDWP_TAG(OBJECT):
         case JDWP_TAG(ARRAY): {
             jobject object;
+            JDI_ASSERT_MSG(request->instance, "Request instance null");
             object = JNI_FUNC_PTR(env,CallObjectMethodA)(env,
                                  request->instance,
                                  request->method,
@@ -511,6 +571,8 @@
         case JDWP_TAG(OBJECT):
         case JDWP_TAG(ARRAY): {
             jobject object;
+            JDI_ASSERT_MSG(request->clazz, "Request clazz null");
+            JDI_ASSERT_MSG(request->instance, "Request instance null");
             object = JNI_FUNC_PTR(env,CallNonvirtualObjectMethodA)(env,
                                            request->instance,
                                            request->clazz,
@@ -607,6 +669,8 @@
     JNIEnv *env;
     jboolean startNow;
     InvokeRequest *request;
+    jbyte options;
+    jbyte invokeType;
 
     JDI_ASSERT(thread);
 
@@ -623,6 +687,9 @@
     if (startNow) {
         request->started = JNI_TRUE;
     }
+    options = request->options;
+    invokeType = request->invokeType;
+
     debugMonitorExit(invokerLock);
 
     if (!startNow) {
@@ -637,7 +704,7 @@
 
         JNI_FUNC_PTR(env,ExceptionClear)(env);
 
-        switch (request->invokeType) {
+        switch (invokeType) {
             case INVOKE_CONSTRUCTOR:
                 invokeConstructor(env, request);
                 break;
@@ -645,7 +712,7 @@
                 invokeStatic(env, request);
                 break;
             case INVOKE_INSTANCE:
-                if (request->options & JDWP_INVOKE_OPTIONS(NONVIRTUAL) ) {
+                if (options & JDWP_INVOKE_OPTIONS(NONVIRTUAL) ) {
                     invokeNonvirtual(env, request);
                 } else {
                     invokeVirtual(env, request);
@@ -723,12 +790,23 @@
     }
 
     /*
+     * At this time, there's no need to retain global references on
+     * arguments since the reply is processed. No one will deal with
+     * this request ID anymore, so we must call deleteGlobalArgumentRefs().
+     *
+     * We cannot delete saved exception or return value references
+     * since otherwise a deleted handle would escape when writing
+     * the response to the stream. Instead, we clean those refs up
+     * after writing the respone.
+     */
+    deleteGlobalArgumentRefs(env, request);
+
+    /*
      * Give up the lock before I/O operation
      */
     debugMonitorExit(invokerLock);
     eventHandler_unlock();
 
-
     if (!detached) {
         outStream_initReply(&out, id);
         (void)outStream_writeValue(env, &out, tag, returnValue);
@@ -736,6 +814,16 @@
         (void)outStream_writeObjectRef(env, &out, exc);
         outStream_sendReply(&out);
     }
+
+    /*
+     * Delete potentially saved global references of return value
+     * and exception
+     */
+    eventHandler_lock(); // for proper lock order
+    debugMonitorEnter(invokerLock);
+    deletePotentiallySavedGlobalRefs(env, request);
+    debugMonitorExit(invokerLock);
+    eventHandler_unlock();
 }
 
 jboolean
diff -r 1179be40f1e3 -r 9f6a0864a734 test/com/sun/jdi/oom/@debuggeeVMOptions
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/com/sun/jdi/oom/@debuggeeVMOptions Mon Mar 21 11:24:09 2016 +0100
@@ -0,0 +1,1 @@
+-Xmx40m
\ No newline at end of file
diff -r 1179be40f1e3 -r 9f6a0864a734 test/com/sun/jdi/oom/OomDebugTest.java
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/com/sun/jdi/oom/OomDebugTest.java Mon Mar 21 11:24:09 2016 +0100
@@ -0,0 +1,417 @@
+/*
+ * Copyright (c) 2016 Red Hat Inc.
+ *
+ * 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 8153711
+ *  @summary JDWP: Memory Leak (global references not deleted after invokeMethod).
+ *
+ *  @author Severin Gehwolf <[hidden email]>
+ *
+ *  @library ..
+ *  @run build TestScaffold VMConnection TargetListener TargetAdapter
+ *  @run compile -g OomDebugTest.java
+ *  @run shell OomDebugTestSetup.sh
+ *  @run main OomDebugTest OomDebugTestTarget test1
+ *  @run main OomDebugTest OomDebugTestTarget test2
+ *  @run main OomDebugTest OomDebugTestTarget test3
+ *  @run main OomDebugTest OomDebugTestTarget test4
+ *  @run main OomDebugTest OomDebugTestTarget test5
+ */
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileNotFoundException;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Properties;
+import java.util.Set;
+
+import com.sun.jdi.ArrayReference;
+import com.sun.jdi.ArrayType;
+import com.sun.jdi.ClassType;
+import com.sun.jdi.Field;
+import com.sun.jdi.InvocationException;
+import com.sun.jdi.Method;
+import com.sun.jdi.ObjectReference;
+import com.sun.jdi.ReferenceType;
+import com.sun.jdi.StackFrame;
+import com.sun.jdi.VMOutOfMemoryException;
+import com.sun.jdi.Value;
+import com.sun.jdi.event.BreakpointEvent;
+import com.sun.jdi.event.ExceptionEvent;
+
+/***************** Target program **********************/
+
+class OomDebugTestTarget {
+
+    OomDebugTestTarget() {
+        System.out.println("DEBUG: invoked constructor");
+    }
+    static class FooCls {
+        @SuppressWarnings("unused")
+        private byte[] bytes = new byte[3000000];
+    };
+
+    FooCls fooCls = new FooCls();
+    byte[] byteArray = new byte[0];
+
+    void testMethod(FooCls foo) {
+        System.out.println("DEBUG: invoked 'void testMethod(FooCls)', foo == " + foo);
+    }
+
+    void testPrimitive(byte[] foo) {
+        System.out.println("DEBUG: invoked 'void testPrimitive(byte[])', foo == " + foo);
+    }
+
+    byte[] testPrimitiveArrRetval() {
+        System.out.println("DEBUG: invoked 'byte[] testPrimitiveArrRetval()'");
+        return new byte[3000000];
+    }
+
+    FooCls testFooClsRetval() {
+        System.out.println("DEBUG: invoked 'FooCls testFooClsRetval()'");
+        return new FooCls();
+    }
+
+    public void entry() {}
+
+    public static void main(String[] args){
+        System.out.println("DEBUG: OomDebugTestTarget.main");
+        new OomDebugTestTarget().entry();
+    }
+}
+
+/***************** Test program ************************/
+
+public class OomDebugTest extends TestScaffold {
+
+    private static final String[] ALL_TESTS = new String[] {
+            "test1", "test2", "test3", "test4", "test5"
+    };
+    private static final Set<String> ALL_TESTS_SET = new HashSet<String>();
+    static {
+        ALL_TESTS_SET.addAll(Arrays.asList(ALL_TESTS));
+    }
+    private static final String TEST_CLASSES = System.getProperty("test.classes", ".");
+    private static final File RESULT_FILE = new File(TEST_CLASSES, "results.properties");
+    private static final String LAST_TEST = ALL_TESTS[ALL_TESTS.length - 1];
+    private ReferenceType targetClass;
+    private ObjectReference thisObject;
+    private int failedTests;
+    private final String testMethod;
+
+    public OomDebugTest(String[] args) {
+        super(args);
+        if (args.length != 2) {
+            throw new RuntimeException("Wrong number of command-line arguments specified.");
+        }
+        this.testMethod = args[1];
+    }
+
+    @Override
+    protected void runTests() throws Exception {
+        try {
+            addListener(new TargetAdapter() {
+
+                @Override
+                public void exceptionThrown(ExceptionEvent event) {
+                    String name = event.exception().referenceType().name();
+                    System.err.println("DEBUG: Exception thrown in debuggee was: " + name);
+                }
+            });
+            /*
+             * Get to the top of entry()
+             * to determine targetClass and mainThread
+             */
+            BreakpointEvent bpe = startTo("OomDebugTestTarget", "entry", "()V");
+            targetClass = bpe.location().declaringType();
+
+            mainThread = bpe.thread();
+
+            StackFrame frame = mainThread.frame(0);
+            thisObject = frame.thisObject();
+            java.lang.reflect.Method m = findTestMethod();
+            m.invoke(this);
+        } catch (NoSuchMethodException e) {
+            e.printStackTrace();
+            failure();
+        } catch (SecurityException e) {
+            e.printStackTrace();
+            failure();
+        }
+        /*
+         * resume the target, listening for events
+         */
+        listenUntilVMDisconnect();
+    }
+
+    private java.lang.reflect.Method findTestMethod()
+            throws NoSuchMethodException, SecurityException {
+        return OomDebugTest.class.getDeclaredMethod(testMethod);
+    }
+
+    private void failure() {
+        failedTests++;
+    }
+
+    /*
+     * Test case: Object reference as method parameter.
+     */
+    @SuppressWarnings("unused") // called via reflection
+    private void test1() throws Exception {
+        System.out.println("DEBUG: ------------> Running test1");
+        try {
+            Field field = targetClass.fieldByName("fooCls");
+            ClassType clsType = (ClassType)field.type();
+            Method constructor = getConstructorForClass(clsType);
+            for (int i = 0; i < 15; i++) {
+                @SuppressWarnings({ "rawtypes", "unchecked" })
+                ObjectReference objRef = clsType.newInstance(mainThread,
+                                                             constructor,
+                                                             new ArrayList(0),
+                                                             ObjectReference.INVOKE_NONVIRTUAL);
+                if (objRef.isCollected()) {
+                    System.out.println("DEBUG: Object got GC'ed before we can use it. NO-OP.");
+                    continue;
+                }
+                invoke("testMethod", "(LOomDebugTestTarget$FooCls;)V", objRef);
+            }
+        } catch (InvocationException e) {
+            handleFailure(e);
+        }
+    }
+
+    /*
+     * Test case: Array reference as method parameter.
+     */
+    @SuppressWarnings("unused") // called via reflection
+    private void test2() throws Exception {
+        System.out.println("DEBUG: ------------> Running test2");
+        try {
+            Field field = targetClass.fieldByName("byteArray");
+            ArrayType arrType = (ArrayType)field.type();
+
+            for (int i = 0; i < 15; i++) {
+                ArrayReference byteArrayVal = arrType.newInstance(3000000);
+                if (byteArrayVal.isCollected()) {
+                    System.out.println("DEBUG: Object got GC'ed before we can use it. NO-OP.");
+                    continue;
+                }
+                invoke("testPrimitive", "([B)V", byteArrayVal);
+            }
+        } catch (VMOutOfMemoryException e) {
+            defaultHandleOOMFailure(e);
+        }
+    }
+
+    /*
+     * Test case: Array reference as return value.
+     */
+    @SuppressWarnings("unused") // called via reflection
+    private void test3() throws Exception {
+        System.out.println("DEBUG: ------------> Running test3");
+        try {
+            for (int i = 0; i < 15; i++) {
+                invoke("testPrimitiveArrRetval",
+                       "()[B",
+                       Collections.EMPTY_LIST,
+                       vm().mirrorOfVoid());
+            }
+        } catch (InvocationException e) {
+            handleFailure(e);
+        }
+    }
+
+    /*
+     * Test case: Object reference as return value.
+     */
+    @SuppressWarnings("unused") // called via reflection
+    private void test4() throws Exception {
+        System.out.println("DEBUG: ------------> Running test4");
+        try {
+            for (int i = 0; i < 15; i++) {
+                invoke("testFooClsRetval",
+                       "()LOomDebugTestTarget$FooCls;",
+                       Collections.EMPTY_LIST,
+                       vm().mirrorOfVoid());
+            }
+        } catch (InvocationException e) {
+            handleFailure(e);
+        }
+    }
+
+    /*
+     * Test case: Constructor
+     */
+    @SuppressWarnings({ "unused", "unchecked", "rawtypes" }) // called via reflection
+    private void test5() throws Exception {
+        System.out.println("DEBUG: ------------> Running test5");
+        try {
+            ClassType type = (ClassType)thisObject.type();
+            for (int i = 0; i < 15; i++) {
+                type.newInstance(mainThread,
+                                 findMethod(targetClass, "<init>", "()V"),
+                                 new ArrayList(0),
+                                 ObjectReference.INVOKE_NONVIRTUAL);
+            }
+        } catch (InvocationException e) {
+            handleFailure(e);
+        }
+    }
+
+    private Method getConstructorForClass(ClassType clsType) {
+        List<Method> methods = clsType.methodsByName("<init>");
+        if (methods.size() != 1) {
+            throw new RuntimeException("FAIL. Expected only one, the default, constructor");
+        }
+        return methods.get(0);
+    }
+
+    private void handleFailure(InvocationException e) {
+        // There is no good way to see the OOME diagnostic message in the target since the
+        // TestScaffold might throw an exception while trying to print the stack trace. I.e
+        // it might get a a VMDisconnectedException before the stack trace printing finishes.
+        System.err.println("FAILURE: InvocationException thrown. Trying to determine cause...");
+        defaultHandleOOMFailure(e);
+    }
+
+    private void defaultHandleOOMFailure(Exception e) {
+        e.printStackTrace();
+        failure();
+    }
+
+    @SuppressWarnings({ "rawtypes", "unchecked" })
+    void invoke(String methodName, String methodSig, Value value)
+            throws Exception {