4858370: JDWP: Memory Leak: GlobalRefs never deleted when processing invokeMethod command

Summary: Delete global references in invoker_completeInvokeRequest()
Reviewed-by: sspitsyn

src/share/back/invoker.c

@@ -211,6 +211,47 @@ createGlobalRefs(JNIEnv *env, InvokeRequest *request)
     return error;
 }
 
+/*
+ * Delete global references from the request which got put there before a
+ * invoke request was carried out. See fillInvokeRequest() and invoker invoke*()
+ * impls.
+ */
+static void
+deleteGlobalRefs(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);
+    }
+    /* Delete potentially saved return values */
+    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));
+        }
+    }
+}
+
 static jvmtiError
 fillInvokeRequest(JNIEnv *env, InvokeRequest *request,
                   jbyte invokeType, jbyte options, jint id,
@@ -736,6 +777,13 @@ invoker_completeInvokeRequest(jthread thread)
         (void)outStream_writeObjectRef(env, &out, exc);
         outStream_sendReply(&out);
     }
+
+    /*
+     * 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 deleteGlobalRefs().
+     */
+    deleteGlobalRefs(env, request);
 }
 
 jboolean

test/com/sun/jdi/oom/@debuggeeVMOptions

+-Xmx40m
\ No newline at end of file

test/com/sun/jdi/oom/OomDebugTest.java

+/*
+ * 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 4858370
+ *  @summary JDWP: Memory Leak (global references not deleted after invokeMethod).
+ *
+ *  @author Severin Gehwolf <sgehwolf@redhat.com>
+ *
+ *  @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.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+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;
+
+/***************** 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 int TOTAL_TESTS = 1;
+    private ReferenceType targetClass;
+    private ObjectReference thisObject;
+    private int failedTests;
+    private final String testMethodName;
+
+    public OomDebugTest(String[] args) {
+        super(args);
+        if (args.length != 2) {
+            throw new RuntimeException("Test failed unexpectedly.");
+        }
+        testMethodName = args[1];
+    }
+
+    @Override
+    protected void runTests() throws Exception {
+        try {
+            /*
+             * 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();
+        }
+    }
+
+    private java.lang.reflect.Method findTestMethod()
+            throws NoSuchMethodException, SecurityException {
+        return OomDebugTest.class.getDeclaredMethod(testMethodName);
+    }
+
+    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 " + testMethodName);
+        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);
+                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 " + testMethodName);
+        try {
+            Field field = targetClass.fieldByName("byteArray");
+            ArrayType arrType = (ArrayType)field.type();
+
+            for (int i = 0; i < 15; i++) {
+                ArrayReference byteArrayVal = arrType.newInstance(3000000);
+                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 " + testMethodName);
+        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 " + testMethodName);
+        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 " + testMethodName);
+        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 caused by OOM");
+        defaultHandleOOMFailure(e);
+    }
+
+    private void defaultHandleOOMFailure(Exception e) {
+        e.printStackTrace();
+        failure();
+    }
+
+    @SuppressWarnings({ "rawtypes", "unchecked" })
+    void invoke(String methodName, String methodSig, Value value)
+            throws Exception {
+        List args = new ArrayList(1);
+        args.add(value);
+        invoke(methodName, methodSig, args, value);
+    }
+
+    void invoke(String methodName,
+                String methodSig,
+                @SuppressWarnings("rawtypes") List args,
+                Value value) throws Exception {
+        Method method = findMethod(targetClass, methodName, methodSig);
+        if ( method == null) {
+            failure("FAILED: Can't find method: "
+                    + methodName  + " for class = " + targetClass);
+            return;
+        }
+        invoke(method, args, value);
+    }
+
+    @SuppressWarnings({ "rawtypes", "unchecked" })
+    void invoke(Method method, List args, Value value) throws Exception {
+        thisObject.invokeMethod(mainThread, method, args, 0);
+        System.out.println("DEBUG: Done invoking method via debugger.");
+    }
+
+    Value fieldValue(String fieldName) {
+        Field field = targetClass.fieldByName(fieldName);
+        return thisObject.getValue(field);
+    }
+
+    public static void main(String[] args) throws Exception {
+        OomDebugTest oomTest = new OomDebugTest(args);
+        oomTest.startTests();
+        if (oomTest.failedTests > 0) {
+            throw new RuntimeException(oomTest.failedTests
+                                       + " of " + TOTAL_TESTS + " test(s) failed.");
+        }
+        System.out.println("All " + TOTAL_TESTS + " tests passed.");
+    }
+
+}

test/com/sun/jdi/oom/OomDebugTestSetup.sh

+#!/bin/sh
+#
+# 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.
+#
+
+
+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
+
+cp ${TESTSRC}/@debuggeeVMOptions ${TESTCLASSES}/