[8u] RFR(xs): 8068042: Check jdk/src/share/native/sun/misc/URLClassPath.c for JNI pending exception

[Jiangli Zhou]

Hi Calvin,

Looks ok.

Thanks,
Jiangli

> On Dec 1, 2015, at 1:05 PM, Calvin Cheung <calvin.cheung at oracle.com> wrote:
> 
> Hi Jiangli, David, Ioi,
> 
> Thank you for the review.
> 
> Here's an updated webrev:
> http://cr.openjdk.java.net/~ccheung/8068042_8u/webrev.01/
> 
> Basically, the JNU_ThrowOutOfMemoryError call has been replaced with a comment.
> 
> thanks,
> Calvin
> 
> On 11/30/15, 8:52 PM, Ioi Lam wrote:
>> 
>> 
>> On 11/30/15 8:21 PM, David Holmes wrote:
>>> On 1/12/2015 12:10 PM, Jiangli Zhou wrote:
>>>> Hi Calvin,
>>>> 
>>>> I see getUTF() throws OutOfMemoryError if malloc fails before returning NULL. Do you know if there is any case that getUTF() returns NULL without throwing any exception? The usage of getUTF() and exception handling don’t seem to be consistent in existing idk code. Some places check the NULL return value without throwing a new exception, while the other places throws a new OOM without checking for pending exception (like the code you are fixing). Maybe the callee does not need to re-throw the exception for the NULL return value case, and you can add a comment saying “an OutOfMemoryError” has already been thrown by getUTF()?
>>> 
>>> According to classloader.c:
>>> 
>>> /* Convert java string to UTF char*. Use local buffer if possible,
>>>   otherwise malloc new memory. Returns null IFF malloc failed. */
>>> 
>>> So NULL implies OOME has been thrown. I'd suggest removing the new code and the JNU_ThrowOutOfMemory with a comment as Jiangli suggested.
>>> 
>>> Also note we have the same kinds of patterns in classLoader.c in 8 and 9.
>>> 
>> I introduced this bug in URLClassPath.c because I copied the code from ClassLoader.c, which had the same pattern, and has been reported in https://bugs.openjdk.java.net/browse/JDK-6952230 (5 years!)
>> 
>> - Ioi
>> 
>>> Thanks,
>>> David
>>> 
>>>> Thanks,
>>>> Jiangli
>>>> 
>>>> 
>>>>> On Nov 30, 2015, at 5:13 PM, Calvin Cheung <calvin.cheung at oracle.com> wrote:
>>>>> 
>>>>> Please review this small fix for adding the check for JNI pending exception in URLClassPath.c.
>>>>> 
>>>>> This fix is only for jdk8u and isn't applicable to 9.
>>>>> 
>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8068042
>>>>>      (unfortunately it is a closed bug)
>>>>> 
>>>>> webrev: http://cr.openjdk.java.net/~ccheung/8068042_8u/webrev.00/
>>>>> 
>>>>> Tests:
>>>>>    JPRT with -testset hotspot
>>>>>    RBT hotspot/test/:hotspot_all,vm.regression.testlist, vm.quick
>>>>> 
>>>>> thanks,
>>>>> Calvin
>>>> 
>>