Looks good. Sorry I missed this issue in reviewing 8162524.

Thanks,
David

On 2/08/2016 10:58 PM, Markus Gronlund wrote:
Greetings,



Please review this small fix to address some new issues seen in testing
where OOM is erroneously being reported:



Bug: https://bugs.openjdk.java.net/browse/JDK-8162945



Changeset:



# HG changeset patch

# User mgronlun

# Date 1470141649 -7200

#      Tue Aug 02 14:40:49 2016 +0200

# Node ID e06e6474ff5f5798f9249aeaa6b33ec6aa021563

# Parent  9672159305d72f5dd430a3edd4b97c4e5bc816e0

[mq]: 8162945



diff --git a/src/jdk.management/share/native/libmanagement_ext/Flag.c
b/src/jdk.management/share/native/libmanagement_ext/Flag.c

--- a/src/jdk.management/share/native/libmanagement_ext/Flag.c

+++ b/src/jdk.management/share/native/libmanagement_ext/Flag.c

@@ -142,7 +142,7 @@

             continue;

         }

-        if (valueObj == NULL) {

+        if (valueObj == NULL && !(globals[i].type ==
JMM_VMGLOBAL_TYPE_JSTRING)) {

             free(globals);

             JNU_ThrowOutOfMemoryError(env, 0);

             return 0;





Summary:



OOM’s have manifested after the following change was integrated:

8162524: src/jdk.management/share/native/libmanagement_ext/Flag.c
doesn't handle JNI exceptions

The following check was then added to
jdk\src\jdk.management\share\native\libmanagement_ext\flags.c:

        if (valueObj == NULL) {
            free(globals);
            JNU_ThrowOutOfMemoryError(env, 0);
            return 0;
        }

However, valueObj is not always the direct result of a failed JNI
allocation, for example:

        case JMM_VMGLOBAL_TYPE_JSTRING:
            valueObj = globals[i].value.l;

The valueObj here (a JSTRING) is coming a VM allocation, more
specifically from :



services/management.cpp jmm_GetVMGlobals()
...
add_global_entry()

} else if (flag->is_ccstr()) {
    Handle str = java_lang_String::create_from_str(flag->get_ccstr(),
CHECK_false); <<--
    global->value.l = (jobject)JNIHandles::make_local(env, str());
    global->type = JMM_VMGLOBAL_TYPE_JSTRING;

For certain ccstr flags, such as  the "HeapDumpFlag" for example, the
ccstr() is NULL (i.e. not set).

So returning a NULL is fine here, the JNI NULL check validation needs to
take this into account.



Thanks

Markus

Reply via email to