8258836: JNI local refs exceed capacity getDiagnosticCommandInfo

jerboaa · jerboaa · commit af155fc06863 · 2021-01-25T08:57:56.000Z
Reviewed-by: cjplummer, shade
diff --git a/src/jdk.management/share/native/libmanagement_ext/DiagnosticCommandImpl.c b/src/jdk.management/share/native/libmanagement_ext/DiagnosticCommandImpl.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2013, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2013, 2021, 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
@@ -45,12 +45,21 @@ Java_com_sun_management_internal_DiagnosticCommandImpl_getDiagnosticCommands
   return jmm_interface->GetDiagnosticCommands(env);
 }
 
-#define EXCEPTION_CHECK_AND_FREE(x) do { \
-                                        if ((*env)->ExceptionCheck(env)) { \
-                                            free(x); \
-                                            return NULL; \
-                                        } \
-                                    } while(0)
+//
+// Checks for an exception and if one occurred,
+// pops 'pops' local frames and frees 'x' before
+// returning NULL
+//
+#define POP_EXCEPTION_CHECK_AND_FREE(pops, x) do { \
+                                                  if ((*env)->ExceptionCheck(env)) { \
+                                                      int i; \
+                                                      for (i = 0; i < pops; i++) { \
+                                                          (*env)->PopLocalFrame(env, NULL); \
+                                                      } \
+                                                      free(x); \
+                                                      return NULL; \
+                                                  } \
+                                              } while(0)
 
 jobject getDiagnosticCommandArgumentInfoArray(JNIEnv *env, jstring command,
                                               int num_arg) {
@@ -73,30 +82,29 @@ jobject getDiagnosticCommandArgumentInfoArray(JNIEnv *env, jstring command,
                                                    dcmd_arg_info_array);
   dcmdArgInfoCls = (*env)->FindClass(env,
                                      "com/sun/management/internal/DiagnosticCommandArgumentInfo");
-  if ((*env)->ExceptionCheck(env)) {
-    free(dcmd_arg_info_array);
-    return NULL;
-  }
+  POP_EXCEPTION_CHECK_AND_FREE(0, dcmd_arg_info_array);
 
   result = (*env)->NewObjectArray(env, num_arg, dcmdArgInfoCls, NULL);
   if (result == NULL) {
     free(dcmd_arg_info_array);
     return NULL;
   }
   for (i=0; i<num_arg; i++) {
-    jstring jname, jdesc,jtype,jdefStr;
+    // Capacity for 5 local refs: jname, jdesc, jtype, jdefStr and obj
+    (*env)->PushLocalFrame(env, 5);
+    jstring jname, jdesc, jtype, jdefStr;
 
     jname = (*env)->NewStringUTF(env,dcmd_arg_info_array[i].name);
-    EXCEPTION_CHECK_AND_FREE(dcmd_arg_info_array);
+    POP_EXCEPTION_CHECK_AND_FREE(1, dcmd_arg_info_array);
 
     jdesc = (*env)->NewStringUTF(env,dcmd_arg_info_array[i].description);
-    EXCEPTION_CHECK_AND_FREE(dcmd_arg_info_array);
+    POP_EXCEPTION_CHECK_AND_FREE(1, dcmd_arg_info_array);
 
     jtype = (*env)->NewStringUTF(env,dcmd_arg_info_array[i].type);
-    EXCEPTION_CHECK_AND_FREE(dcmd_arg_info_array);
+    POP_EXCEPTION_CHECK_AND_FREE(1, dcmd_arg_info_array);
 
     jdefStr = (*env)->NewStringUTF(env, dcmd_arg_info_array[i].default_string);
-    EXCEPTION_CHECK_AND_FREE(dcmd_arg_info_array);
+    POP_EXCEPTION_CHECK_AND_FREE(1, dcmd_arg_info_array);
     obj = JNU_NewObjectByName(env,
                               "com/sun/management/internal/DiagnosticCommandArgumentInfo",
                               "(Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ZZZI)V",
@@ -107,11 +115,13 @@ jobject getDiagnosticCommandArgumentInfoArray(JNIEnv *env, jstring command,
                               dcmd_arg_info_array[i].multiple,
                               dcmd_arg_info_array[i].position);
     if (obj == NULL) {
+      (*env)->PopLocalFrame(env, NULL);
       free(dcmd_arg_info_array);
       return NULL;
     }
+    obj = (*env)->PopLocalFrame(env, obj);
     (*env)->SetObjectArrayElement(env, result, i, obj);
-    EXCEPTION_CHECK_AND_FREE(dcmd_arg_info_array);
+    POP_EXCEPTION_CHECK_AND_FREE(0, dcmd_arg_info_array);
   }
   free(dcmd_arg_info_array);
   arraysCls = (*env)->FindClass(env, "java/util/Arrays");
@@ -144,51 +154,67 @@ Java_com_sun_management_internal_DiagnosticCommandImpl_getDiagnosticCommandInfo
   jint ret = jmm_interface->GetOptionalSupport(env, &mos);
   jsize num_commands;
   dcmdInfo* dcmd_info_array;
-  jstring jname, jdesc, jimpact;
+  jstring jname, jdesc, jimpact, cmd;
 
   if (commands == NULL) {
       JNU_ThrowNullPointerException(env, "Invalid String Array");
       return NULL;
   }
   num_commands = (*env)->GetArrayLength(env, commands);
+  // Ensure capacity for 2 + num_commands local refs:
+  //  2 => dcmdInfoCls, result
+  //  num_commmands => one obj per command
+  (*env)->PushLocalFrame(env, 2 + num_commands);
   dcmdInfoCls = (*env)->FindClass(env,
                                   "com/sun/management/internal/DiagnosticCommandInfo");
   if ((*env)->ExceptionCheck(env)) {
+    (*env)->PopLocalFrame(env, NULL);
     return NULL;
   }
 
   result = (*env)->NewObjectArray(env, num_commands, dcmdInfoCls, NULL);
   if (result == NULL) {
+      (*env)->PopLocalFrame(env, NULL);
       return NULL;
   }
   if (num_commands == 0) {
+      result = (*env)->PopLocalFrame(env, result);
       /* Handle the 'zero commands' case specially to avoid calling 'malloc()' */
       /* with a zero argument because that may legally return a NULL pointer.  */
       return result;
   }
   dcmd_info_array = (dcmdInfo*) malloc(num_commands * sizeof(dcmdInfo));
   if (dcmd_info_array == NULL) {
+      (*env)->PopLocalFrame(env, NULL);
       JNU_ThrowOutOfMemoryError(env, NULL);
       return NULL;
   }
   jmm_interface->GetDiagnosticCommandInfo(env, commands, dcmd_info_array);
   for (i=0; i<num_commands; i++) {
+      // Ensure capacity for 6 + 3 local refs:
+      //  6 => jname, jdesc, jimpact, cmd, args, obj
+      //  3 => permission class, name, action
+      (*env)->PushLocalFrame(env, 6 + 3);
+
+      cmd = (*env)->GetObjectArrayElement(env, commands, i);
       args = getDiagnosticCommandArgumentInfoArray(env,
-                                                   (*env)->GetObjectArrayElement(env,commands,i),
+                                                   cmd,
                                                    dcmd_info_array[i].num_arguments);
       if (args == NULL) {
+          (*env)->PopLocalFrame(env, NULL);
+          (*env)->PopLocalFrame(env, NULL);
           free(dcmd_info_array);
           return NULL;
       }
 
       jname = (*env)->NewStringUTF(env,dcmd_info_array[i].name);
-      EXCEPTION_CHECK_AND_FREE(dcmd_info_array);
+      POP_EXCEPTION_CHECK_AND_FREE(2, dcmd_info_array);
 
       jdesc = (*env)->NewStringUTF(env,dcmd_info_array[i].description);
-      EXCEPTION_CHECK_AND_FREE(dcmd_info_array);
+      POP_EXCEPTION_CHECK_AND_FREE(2, dcmd_info_array);
 
       jimpact = (*env)->NewStringUTF(env,dcmd_info_array[i].impact);
-      EXCEPTION_CHECK_AND_FREE(dcmd_info_array);
+      POP_EXCEPTION_CHECK_AND_FREE(2, dcmd_info_array);
 
       obj = JNU_NewObjectByName(env,
                                 "com/sun/management/internal/DiagnosticCommandInfo",
@@ -200,13 +226,17 @@ Java_com_sun_management_internal_DiagnosticCommandImpl_getDiagnosticCommandInfo
                                 dcmd_info_array[i].enabled,
                                 args);
       if (obj == NULL) {
+          (*env)->PopLocalFrame(env, NULL);
+          (*env)->PopLocalFrame(env, NULL);
           free(dcmd_info_array);
           return NULL;
       }
+      obj = (*env)->PopLocalFrame(env, obj);
 
       (*env)->SetObjectArrayElement(env, result, i, obj);
-      EXCEPTION_CHECK_AND_FREE(dcmd_info_array);
+      POP_EXCEPTION_CHECK_AND_FREE(1, dcmd_info_array);
   }
+  result = (*env)->PopLocalFrame(env, result);
   free(dcmd_info_array);
   return result;
 }
diff --git a/test/jdk/com/sun/management/DiagnosticCommandMBean/DcmdMBeanTestCheckJni.java b/test/jdk/com/sun/management/DiagnosticCommandMBean/DcmdMBeanTestCheckJni.java
@@ -0,0 +1,84 @@
+/*
+ * Copyright (c) 2021, 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     8258836
+ * @summary JNI local refs exceed capacity getDiagnosticCommandInfo
+ * @library /test/lib
+ * @run main/othervm DcmdMBeanTestCheckJni
+ */
+
+import java.lang.management.ManagementFactory;
+import javax.management.MBeanServer;
+import javax.management.ObjectName;
+import javax.management.MBeanServerConnection;
+import javax.management.remote.JMXConnectorFactory;
+import javax.management.remote.JMXConnector;
+import javax.management.remote.JMXConnectorServerFactory;
+import javax.management.remote.JMXServiceURL;
+import javax.management.remote.JMXConnectorServer;
+
+import jdk.test.lib.process.OutputAnalyzer;
+import jdk.test.lib.process.ProcessTools;
+
+public class DcmdMBeanTestCheckJni {
+
+    public static void main(String[] args) throws Exception {
+        OutputAnalyzer out = ProcessTools.executeTestJvm(
+            "-Xcheck:jni",
+            DcmdMBeanRunner.class.getName());
+        out.shouldNotMatch("WARNING: JNI local refs: \\d+, exceeds capacity: \\d+")
+           .shouldContain("DcmdMBeanRunner COMPLETE")
+           .shouldHaveExitValue(0);
+    }
+
+}
+
+class DcmdMBeanRunner {
+
+    private static final String HOTSPOT_DIAGNOSTIC_MXBEAN_NAME =
+        "com.sun.management:type=DiagnosticCommand";
+
+    public static void main(String[] args) throws Exception {
+        MBeanServer mbs = ManagementFactory.getPlatformMBeanServer();
+        JMXServiceURL url = new JMXServiceURL("rmi", null, 0);
+        JMXConnectorServer cs = null;
+        JMXConnector cc = null;
+        try {
+            cs = JMXConnectorServerFactory.newJMXConnectorServer(url, null, mbs);
+            cs.start();
+            JMXServiceURL addr = cs.getAddress();
+            cc = JMXConnectorFactory.connect(addr);
+            MBeanServerConnection mbsc = cc.getMBeanServerConnection();
+            ObjectName name = new ObjectName(HOTSPOT_DIAGNOSTIC_MXBEAN_NAME);
+            System.out.println("DiagnosticCommand MBean: " + name);
+            System.out.println("DcmdMBeanRunner COMPLETE");
+        } finally {
+            try {
+                cc.close();
+                cs.stop();
+            } catch (Exception e) { /* ignored */ }
+        }
+    }
+}
