RFR: 7180557 - InetAddress.getLocalHost throws UnknownHostException on java7u5 on OSX
Rob McKenna
rob.mckenna at oracle.com
Mon Oct 7 09:38:35 PDT 2013
Heh, you just beat me to the punch.
-Rob
On 07/10/13 17:34, Dmitry Samersoff wrote:
> Rob,
>
> Existing code uses if (JVM_GetHostName(myhostname, NI_MAXHOST)) so I
> withdraw my last comments. Please, don't change it!!!
>
> -Dmitry
>
> On 2013-10-07 20:30, Rob McKenna wrote:
>> Thanks Dmitry! I'll correct that nipick pre-push.
>>
>> -Rob
>>
>> On 07/10/13 16:47, Dmitry Samersoff wrote:
>>> Rob,
>>>
>>> This version of your fix looks good for me.
>>>
>>>
>>> Inet4AddressImpl.c:
>>>
>>> Thumbs up.
>>>
>>> Inet6AddressImpl.c:
>>>
>>> Thumbs up.
>>>
>>> 173 if (JVM_GetHostName(myhostname, NI_MAXHOST)) {
>>>
>>> Nitpicking, explicit == -1 would be better here.
>>>
>>>
>>>> Actually, can you tell me why you'd rather not
>>>> include ipv6 loopbacks at all?
>>> If interface don't have ipv6 address but we have ipv6 loopback it most
>>> likely means that ipv6 is not functioning properly.
>>>
>>> -Dmitry
>>>
>>>
>>> On 2013-10-04 19:03, Rob McKenna wrote:
>>>> Hi Dmitry,
>>>>
>>>> Thanks a lot for the comprehensive review. W.r.t. line 210, I agree
>>>> there is a problem with the logic, but I'd like to suggest an
>>>> alternative solution:
>>>>
>>>> - If addrs4 >= 1, then we will always be including loopback addresses in
>>>> the list. Since the idea of this extra condition is to exclude loopback
>>>> interfaces from the list unless they're the only available addresses, I
>>>> would suggest "(addrs4 == numV4Loopback && addrs6 == numV6Loopback)"
>>>> instead.
>>>>
>>>> - On the very limited chance that a user has a machine with only an ipv6
>>>> loopback configured (unlikely, I'd agree) it probably makes sense to
>>>> leave it in there. Actually, can you tell me why you'd rather not
>>>> include ipv6 loopbacks at all?
>>>>
>>>> New webrev at:
>>>>
>>>> http://cr.openjdk.java.net/~robm/7180557/webrev.04/
>>>>
>>>> -Rob
>>>>
>>>> On 21/09/13 12:20, Dmitry Samersoff wrote:
>>>>> Rob,
>>>>>
>>>>> See below -
>>>>> ll. 210 have to be fixed, other comments is just an opinion.
>>>>>
>>>>>
>>>>> Inet4AddressImpl.c:
>>>>>
>>>>> ll.132 - it might be better to move initialization to a separate
>>>>> function, the same way as in Inet6 - I really like this idea.
>>>>>
>>>>> Inet6AddressImpl.c
>>>>>
>>>>>
>>>>> ll. 126.
>>>>>
>>>>> it's better to move static int initialized into initializeInetClasses()
>>>>> and don't check it ll. 282.
>>>>>
>>>>>
>>>>> ll. 170
>>>>>
>>>>> it looks like rest of the code uses NI_MAXHOST also we have to check
>>>>> results of JVM_GetHostName if it returns -1 it's probably better to
>>>>> bailout immediately.
>>>>>
>>>>>
>>>>> ll. 193 and below - wrong indent
>>>>>
>>>>> 4)
>>>>>
>>>>> ll. 210
>>>>>
>>>>> we can have more than one v4 address so
>>>>>
>>>>> if (addrs4 >= 1)
>>>>>
>>>>> have to be here.
>>>>>
>>>>> *.
>>>>>
>>>>> Your include ipv6 loopback in the list if interface has no ipv4
>>>>> addresses, I'm not sure this logic is correct. On my opinion it's
>>>>> better
>>>>> to never include ipv6 loopbacks.
>>>>>
>>>>> *.
>>>>>
>>>>> Is it better to rename:
>>>>> includeLoopback => includeLoopbacks
>>>>>
>>>>>
>>>>> ll. 218
>>>>>
>>>>> It might be better to calculate arraySize under if at ll. 210 to make
>>>>> code better readable
>>>>>
>>>>> ll. 236
>>>>>
>>>>> It might be better to split if statement to make it more readable at
>>>>> the
>>>>> cost of duplicating iter = iter->ifa_next; line.
>>>>>
>>>>> e.g.
>>>>>
>>>>> while (iter != NULL) {
>>>>> ...
>>>>>
>>>>> if (family != AF_INET6 and family != AF_INET) {
>>>>> iter = iter->ifa_next;
>>>>> continue;
>>>>> }
>>>>>
>>>>> if ((!includeV6 and family == AF_INET6)) {
>>>>> iter = iter->ifa_next;
>>>>> continue;
>>>>> }
>>>>>
>>>>> if (!includeLoopback and isLoopback) {
>>>>> iter = iter->ifa_next;
>>>>> continue;
>>>>> }
>>>>>
>>>>> if (iter->ifa_name[0] != '\0' && iter->ifa_addr) {
>>>>> // Copy address to Java array
>>>>> ....
>>>>> iter = iter->ifa_next;
>>>>> continue; // redundant just for readability
>>>>> }
>>>>> }
>>>>>
>>>>> ll.244
>>>>>
>>>>> I'm not sure it's good to return partially complete array in case of
>>>>> object allocation fail. Probably you should throw
>>>>>
>>>>> JNU_ThrowOutOfMemoryError(env, "Object allocation failed");
>>>>>
>>>>> -Dmitry
>>>>>
>>>>>
>>>>> On 2013-09-20 18:58, Rob McKenna wrote:
>>>>>> After a brief discussion with Chris, we decided to revert the position
>>>>>> of the call to lookupIfLocalAddrs so as to give the local host primacy
>>>>>> over DNS.
>>>>>>
>>>>>> Latest (and hopefully last) webrev here:
>>>>>>
>>>>>> http://cr.openjdk.java.net/~robm/7180557/webrev.03/
>>>>>>
>>>>>> -Rob
>>>>>>
>>>>>> On 14/09/13 00:00, Rob McKenna wrote:
>>>>>>> Hi Bernd,
>>>>>>>
>>>>>>> I should have said in the context of this bug. What I meant was
>>>>>>> removing AI_CANONNAME doesn't resolve the issue as far as Mac is
>>>>>>> concerned. I.e. I still see the UnknownHostException. In this
>>>>>>> particular case the hostname is not set via the hosts file.
>>>>>>>
>>>>>>> In the latest webrev the call to getifaddrs only occurs if
>>>>>>> getaddrinfo
>>>>>>> fails.
>>>>>>>
>>>>>>> -Rob
>>>>>>>
>>>>>>> On 13/09/13 20:28, Bernd Eckenfels wrote:
>>>>>>>> Am 13.09.2013, 19:32 Uhr, schrieb Rob McKenna
>>>>>>>> <rob.mckenna at oracle.com>:
>>>>>>>>> W.r.t. the use of AI_CANONNAME, this doesn't actually make a
>>>>>>>>> difference in the context of this fix, but is definitely something
>>>>>>>>> that should be looked at. I'll put it on the todo list.
>>>>>>>> I think it does make a difference: If you remove the CANON flag
>>>>>>>> getaddrinfo() will not do DNS lookups when the host is configured to
>>>>>>>> prefer the hosts file (which it should do on Linux and OSX). And so
>>>>>>>> the platform specific lookupIfLocalhost() can be put after the
>>>>>>>> getaddrinfo() (again).
>>>>>>>>
>>>>>>>> I actually think the bug "exists" on all platforms. If getaddrinfo()
>>>>>>>> fails because neighter hosts nor DNS file finds the name this can
>>>>>>>> happen on all platforms. I dont think it helps to add a fallback
>>>>>>>> only
>>>>>>>> on MACOSX (and there is certainly no need to prefer the fallback
>>>>>>>> then).
>>>>>>>>
>>>>>>>> Gruss
>>>>>>>> Bernd
>
More information about the net-dev
mailing list