RFR [8022584] Memory leak in some NetworkInterface methods

[Chris Hegarty]

Looks good to me. Thanks Ivan.

-Chris.

On 08/08/2013 02:33 PM, Ivan Gerasimov wrote:
> Hello Chris!
>
> Here's the update:
> http://cr.openjdk.java.net/~igerasim/8022584/2/webrev/
>
> Thanks for offering the sponsorship!
> Here's the hg-export
> http://cr.openjdk.java.net/~igerasim/2commit/8022584-jdk8-Memleak-in-NetworkInterface.patch
>
>
> Sincerely yours,
> Ivan
>
> On 08.08.2013 12:43, Chris Hegarty wrote:
>> Thanks for taking this Ivan.
>>
>> Can you please make the changes suggested by both David and Alan (
>> simply return NULL/-1/JNI_FALSE, as appropriate, if GetStringUTFChars
>> fails ( returns NULL ), then I will sponsor this change into jdk8 for
>> you.
>>
>> Please post an update webrev to cr.openjdk.java.net.
>>
>> -Chris.
>>
>> On 08/08/2013 06:01 AM, David Holmes wrote:
>>> Ivan,
>>>
>>> On 8/08/2013 2:05 PM, Ivan Gerasimov wrote:
>>>> David, Alan,
>>>>
>>>> I added checking for NULL  results and throwing OOMException if
>>>> necessary.
>>>
>>> You don't need to throw it yourself:
>>>
>>>    JNU_ThrowOutOfMemoryError(env, NULL);
>>>
>>> Assuming a correct VM implementation if NULL is returned then an OOME
>>> should already be pending and will be thrown as soon as your native code
>>> returns to Java.
>>>
>>> Thanks,
>>> David
>>>
>>>> I've also added some spaces along the code to improve indentation.
>>>>
>>>> Would you please review the updated webrev?
>>>> http://washi.ru.oracle.com/~igerasim/webrevs/8022584/1/webrev/
>>>>
>>>> Sincerely yours,
>>>> Ivan
>>>>
>>>>
>>>> On 08.08.2013 5:35, David Holmes wrote:
>>>>> On 8/08/2013 11:20 AM, Ivan Gerasimov wrote:
>>>>>> Thanks Alan!
>>>>>>
>>>>>> I've checked that and it turns out that GetStringUTFChars cannot
>>>>>> return
>>>>>> NULL.
>>>>>> For allocation of the result string it calls AllocateHeap() with the
>>>>>> default EXIT_OOM fail strategy.
>>>>>> Thus, in case of being unable to allocate memory it simply stops VM.
>>>>>
>>>>> Ouch! That is a hotspot bug. GetStringUTFChars is supposed to throw
>>>>> OutOfMemoryError if it has memory issues, not abort the VM!
>>>>>
>>>>> I recommend that you check for NULL and/or a pending exception.
>>>>>
>>>>> David
>>>>>
>>>>>> Sincerely yours,
>>>>>> Ivan
>>>>>>
>>>>>> On 08.08.2013 5:05, Alan Bateman wrote:
>>>>>>> (cc'ing net-dev).
>>>>>>>
>>>>>>> The change looks okay to me. One suggestion (while you are there) is
>>>>>>> to check the return from  GetStringUTFChars so that the name returns
>>>>>>> when it fails with NULL.
>>>>>>>
>>>>>>> -Alan.
>>>>>>>
>>>>>>> On 07/08/2013 17:46, Ivan Gerasimov wrote:
>>>>>>>> Hello everybody!
>>>>>>>>
>>>>>>>> Some methods of NetworkInterface class were reported to leak
>>>>>>>> memory.
>>>>>>>> http://bugs.sun.com/view_bug.do?bug_id=JDK-8022584 (not yet
>>>>>>>> available.)
>>>>>>>>
>>>>>>>> Examples are isUp() and isLoopback().
>>>>>>>>
>>>>>>>> Affected versions of jdk are 6, 7 and 8
>>>>>>>>
>>>>>>>> Would you please review a simple fix that removes the unnecessary
>>>>>>>> allocation?
>>>>>>>> http://cr.openjdk.java.net/~igerasim/8022584/0/webrev/
>>>>>>>>
>>>>>>>> Sincerely yours,
>>>>>>>> Ivan
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>
>>
>