[9] RFR: 8170465, 8170466: JNI exception pending in jni_util.c:190

[David Holmes]

On 7/12/2016 11:04 AM, Naoto Sato wrote:
> Thanks, David. The fix is modified accordingly:
>
> http://cr.openjdk.java.net/~naoto/8170465.8170466/webrev.01/

Thanks for fixing that.

The null checks after the exception checks are now redundant as you can 
only get a null return from those methods if an exception becomes 
pending. But leaving them in as a precaution is okay I guess.

Thanks,
David

> Naoto
>
> On 12/6/16 4:53 PM, David Holmes wrote:
>> Hi Naoto,
>>
>> On 7/12/2016 10:02 AM, Naoto Sato wrote:
>>> Hello,
>>>
>>> Please review the proposed fix to the following issues:
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8170465
>>> https://bugs.openjdk.java.net/browse/JDK-8170466
>>>
>>> The proposed fix is located at:
>>>
>>> http://cr.openjdk.java.net/~naoto/8170465.8170466/webrev.00/
>>>
>>> These two JNI calls (NewStringUTF() and JNU_CallMethodByName()) in
>>> src/java.base/share/native/libjava/jni_util.c can throw an exception.
>>> Those possible exceptions need to be properly checked.
>>
>> JNU_CHECK_EXCEPTION will do a return from the method in which it is
>> placed, so you need to be careful to ensure you do all needed cleanup
>> before that eg:
>>
>>  203                 JNU_CHECK_EXCEPTION(env);
>>  204                 free(str1);
>>
>> needs to be reversed else we won't free str1. Similarly:
>>
>>  210                     JNU_CHECK_EXCEPTION(env);
>>  211                     (*env)->DeleteLocalRef(env, s2);
>>
>> needs to be reversed so we don't leak the local ref. (It is safe to call
>> DeleteLocalRef with an exception pending.)
>>
>> Thanks,
>> David
>>
>>> Naoto