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

[Calvin Cheung]

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