RFR(XXS): 8162945: HotspotDiagnosticMXBean getFlags erroneously reports OutOfMemory

[Daniel D. Daugherty]

On 8/2/16 6:58 AM, 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)) {
>

Might be too late, but I would prefer:

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

which I think is more clear.

Dan


> 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
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20160802/9c211d81/attachment.html>