RFR [8022584] Memory leak in some NetworkInterface methods

[Ivan Gerasimov]

Chris, if it's not too late, I'd like to include a regtest in the fix.

Here's webrev that includes the test:
http://cr.openjdk.java.net/~igerasim/8022584/3/webrev/

The test gets past with the fixed jdk8 and fails with jdk8-b101 and jdk7 
as expected.

Sincerely yours,
Ivan

On 08.08.2013 17:50, Chris Hegarty wrote:
> 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
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>
>>>
>>
>
>