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

[Langer, Christoph]

Hi Naoto,

the new webrev looks ok to me, too. Thanks for fixing.

And I agree to the point that the NULL check can be removed...if you like.

Best regards
Christoph

> -----Original Message-----
> From: core-libs-dev [mailto:core-libs-dev-bounces at openjdk.java.net] On Behalf
> Of David Holmes
> Sent: Mittwoch, 7. Dezember 2016 03:15
> To: Naoto Sato <naoto.sato at oracle.com>; core-libs-dev <core-libs-
> dev at openjdk.java.net>
> Subject: Re: [9] RFR: 8170465, 8170466: JNI exception pending in jni_util.c:190
> 
> 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