RFR : JDK-8162530 : src/jdk.management/share/native/libmanagement_ext/GcInfoBuilder.c doesn't handle JNI exceptions properly

[Staffan Larsen]

Looks good!

Thanks,
/Staffan

> On 18 aug. 2016, at 07:53, Amit Sapre <amit.sapre at oracle.com> wrote:
> 
> Guys,
> I need one more person to review these changes. Kindly help with the review.
> Thanks,
> Amit
> 
> -----Original Message-----
> From: David Holmes 
> Sent: Thursday, August 11, 2016 5:55 AM
> To: Amit Sapre; serviceability-dev
> Subject: Re: RFR : JDK-8162530 : src/jdk.management/share/native/libmanagement_ext/GcInfoBuilder.c doesn't handle JNI exceptions properly
> 
> On 10/08/2016 8:46 PM, Amit Sapre wrote:
>> Hello,
>> Well for some reason, unknown to me, JNI code checker is not 
>> complaining for exception thrown by SetObjectArrayElement. It complains for any pending exceptions before calling SetObjectArrayElement.
>> 
>> I have added exceptions checks after calling SetObjectArrayElement. The updated changes are in this webrev.
>> 
>> http://cr.openjdk.java.net/~sballal/sponsorship/8162530/webrev.02/
> 
> 
> This looks fine to me.
> 
> Thanks,
> David
> 
>> Thanks,
>> Amit
>> 
>> 
>> 
>> -----Original Message-----
>> From: David Holmes
>> Sent: Wednesday, August 03, 2016 5:50 AM
>> To: Amit Sapre; serviceability-dev
>> Subject: Re: RFR : JDK-8162530 : 
>> src/jdk.management/share/native/libmanagement_ext/GcInfoBuilder.c 
>> doesn't handle JNI exceptions properly
>> 
>> Hi Amit,
>> 
>> On 2/08/2016 7:49 PM, Amit Sapre wrote:
>>> Hello,
>>> I have made changes as David suggested.
>>> 
>>> Here is the new webrev link.
>>> http://cr.openjdk.java.net/~hb/sponsorship/8162530/webrev.01/
>> 
>> If the changes are required to pass a JNI code checker then you will also need to check for exceptions from SetObjectArrayElement. While ArrayStoreException should not be possible in this code, I can't tell for certain that the passed in arrays are ensured to have the required lengths.
>> 
>> Thanks,
>> David
>> 
>>> Thanks,
>>> Amit
>>> 
>>> -----Original Message-----
>>> From: David Holmes
>>> Sent: Monday, August 01, 2016 12:33 PM
>>> To: Amit Sapre; serviceability-dev
>>> Subject: Re: RFR : JDK-8162530 :
>>> src/jdk.management/share/native/libmanagement_ext/GcInfoBuilder.c
>>> doesn't handle JNI exceptions properly
>>> 
>>> Hi Amit,
>>> 
>>> On 1/08/2016 4:10 PM, Amit Sapre wrote:
>>>> Hello,
>>>> 
>>>> 
>>>> 
>>>> Please review JNI exception handling related changes.
>>>> 
>>>> Bug id : https://bugs.openjdk.java.net/browse/JDK-8162530
>>>> 
>>>> Webrev :
>>>> http://cr.openjdk.java.net/~hb/sponsorship/8162530/webrev.00/
>>> 
>>> Sorry but that's wrong way to fix this. We should never just blindly clear exceptions and continue as if they never happened. If an exception is pending after one of these JNI calls the method should return immediately and allow the exception to propagate to the java code.
>>> 
>>> Thanks,
>>> David
>>> 
>>>> 
>>>> 
>>>> Thanks,
>>>> 
>>>> Amit
>>>>