Re: 8143397: It looks like InetAddress.isReachable(timeout) works incorrectly
Hi Rob, change looks fine and handles the MS idiosyncrasies neatly change works fine ... consistent responses and failing test returns expected results regards Mark On 09/12/2015 01:44, Rob McKenna wrote: The intention of the 2nd revision of the fix is to make the undocumented 1000ms problem a non issue. If a user calls this function with a timeout of 200ms that timeout is automatically substituted for 1000ms in the IcmpSendEcho call. Once the response is received its RTT is checked to make sure it is lower than 200ms. If not the method returns false. That being the case though the call could potentially take up to 1000ms to complete where a host is not reachable even though a smaller timeout is specified. The return value of isReachable() will correctly say whether the host was reachable or not within the actual timeout specified by the user however. In order to avoid the older (and less reliable) way of testing reachability I feel that this small corner case is a worthwhile tradeoff. -Rob On 09/12/15 01:31, Xuelei Fan wrote: Is it nice to say in the spec that it is not reliable if the timeout is too small? At least 1000ms timeout by default may be not acceptable in some circumstances. Xuelei On 12/9/2015 12:31 AM, Rob McKenna wrote: Testing has shown that when a timeout < 1000ms is specified the IcmpSendEcho calls fail (apparently) randomly. Once the timeout is 1000ms or greater it works as expected. Therefore I've updated the fix to use 1000ms as a minimum. The existing logic ensures that the ttl is less than the specified timeout in any case: http://cr.openjdk.java.net/~robm/8143397/webrev.02/ -Rob On 01/12/15 14:59, Rob McKenna wrote: It appears that there is an undocumented minimum timeout in the IcmpSendEcho* functions. If the timeout parameter is set to a number below this minimum timeout it is effectively ignored. Thus if you wanted to ensure that you could ping a particular host within a certain timeout less than the undocumented minimum, you could potentially receive a false positive. (i.e. if you set the timeout to 20ms but the ping takes 30ms, IcmpSendEcho will still succeed) The following fix checks the icmp reply packet and compares the round trip time to the requested timeout parameter before deciding whether the call was successful or not: http://cr.openjdk.java.net/~robm/8143397/webrev.01/ -Rob
Re: 8143397: It looks like InetAddress.isReachable(timeout) works incorrectly
On 12/9/2015 9:44 AM, Rob McKenna wrote: > The intention of the 2nd revision of the fix is to make the undocumented > 1000ms problem a non issue. > > If a user calls this function with a timeout of 200ms that timeout is > automatically substituted for 1000ms in the IcmpSendEcho call. Once the > response is received its RTT is checked to make sure it is lower than > 200ms. If not the method returns false. > OK. It makes sense to me. Thanks, Xuelei > That being the case though the call could potentially take up to 1000ms > to complete where a host is not reachable even though a smaller timeout > is specified. The return value of isReachable() will correctly say > whether the host was reachable or not within the actual timeout > specified by the user however. > > In order to avoid the older (and less reliable) way of testing > reachability I feel that this small corner case is a worthwhile tradeoff. > > -Rob > > On 09/12/15 01:31, Xuelei Fan wrote: >> Is it nice to say in the spec that it is not reliable if the timeout is >> too small? At least 1000ms timeout by default may be not acceptable in >> some circumstances. >> >> Xuelei >> >> On 12/9/2015 12:31 AM, Rob McKenna wrote: >>> Testing has shown that when a timeout < 1000ms is specified the >>> IcmpSendEcho calls fail (apparently) randomly. Once the timeout is >>> 1000ms or greater it works as expected. Therefore I've updated the fix >>> to use 1000ms as a minimum. The existing logic ensures that the ttl is >>> less than the specified timeout in any case: >>> >>> http://cr.openjdk.java.net/~robm/8143397/webrev.02/ >>> >>> -Rob >>> >>> On 01/12/15 14:59, Rob McKenna wrote: It appears that there is an undocumented minimum timeout in the IcmpSendEcho* functions. If the timeout parameter is set to a number below this minimum timeout it is effectively ignored. Thus if you wanted to ensure that you could ping a particular host within a certain timeout less than the undocumented minimum, you could potentially receive a false positive. (i.e. if you set the timeout to 20ms but the ping takes 30ms, IcmpSendEcho will still succeed) The following fix checks the icmp reply packet and compares the round trip time to the requested timeout parameter before deciding whether the call was successful or not: http://cr.openjdk.java.net/~robm/8143397/webrev.01/ -Rob >>
Re: 8143397: It looks like InetAddress.isReachable(timeout) works incorrectly
The intention of the 2nd revision of the fix is to make the undocumented 1000ms problem a non issue. If a user calls this function with a timeout of 200ms that timeout is automatically substituted for 1000ms in the IcmpSendEcho call. Once the response is received its RTT is checked to make sure it is lower than 200ms. If not the method returns false. That being the case though the call could potentially take up to 1000ms to complete where a host is not reachable even though a smaller timeout is specified. The return value of isReachable() will correctly say whether the host was reachable or not within the actual timeout specified by the user however. In order to avoid the older (and less reliable) way of testing reachability I feel that this small corner case is a worthwhile tradeoff. -Rob On 09/12/15 01:31, Xuelei Fan wrote: Is it nice to say in the spec that it is not reliable if the timeout is too small? At least 1000ms timeout by default may be not acceptable in some circumstances. Xuelei On 12/9/2015 12:31 AM, Rob McKenna wrote: Testing has shown that when a timeout < 1000ms is specified the IcmpSendEcho calls fail (apparently) randomly. Once the timeout is 1000ms or greater it works as expected. Therefore I've updated the fix to use 1000ms as a minimum. The existing logic ensures that the ttl is less than the specified timeout in any case: http://cr.openjdk.java.net/~robm/8143397/webrev.02/ -Rob On 01/12/15 14:59, Rob McKenna wrote: It appears that there is an undocumented minimum timeout in the IcmpSendEcho* functions. If the timeout parameter is set to a number below this minimum timeout it is effectively ignored. Thus if you wanted to ensure that you could ping a particular host within a certain timeout less than the undocumented minimum, you could potentially receive a false positive. (i.e. if you set the timeout to 20ms but the ping takes 30ms, IcmpSendEcho will still succeed) The following fix checks the icmp reply packet and compares the round trip time to the requested timeout parameter before deciding whether the call was successful or not: http://cr.openjdk.java.net/~robm/8143397/webrev.01/ -Rob
Re: 8143397: It looks like InetAddress.isReachable(timeout) works incorrectly
Is it nice to say in the spec that it is not reliable if the timeout is too small? At least 1000ms timeout by default may be not acceptable in some circumstances. Xuelei On 12/9/2015 12:31 AM, Rob McKenna wrote: > Testing has shown that when a timeout < 1000ms is specified the > IcmpSendEcho calls fail (apparently) randomly. Once the timeout is > 1000ms or greater it works as expected. Therefore I've updated the fix > to use 1000ms as a minimum. The existing logic ensures that the ttl is > less than the specified timeout in any case: > > http://cr.openjdk.java.net/~robm/8143397/webrev.02/ > > -Rob > > On 01/12/15 14:59, Rob McKenna wrote: >> It appears that there is an undocumented minimum timeout in the >> IcmpSendEcho* functions. If the timeout parameter is set to a number >> below this minimum timeout it is effectively ignored. Thus if you wanted >> to ensure that you could ping a particular host within a certain timeout >> less than the undocumented minimum, you could potentially receive a >> false positive. (i.e. if you set the timeout to 20ms but the ping takes >> 30ms, IcmpSendEcho will still succeed) >> >> The following fix checks the icmp reply packet and compares the round >> trip time to the requested timeout parameter before deciding whether the >> call was successful or not: >> >> http://cr.openjdk.java.net/~robm/8143397/webrev.01/ >> >> -Rob
Re: 8143397: It looks like InetAddress.isReachable(timeout) works incorrectly
Testing has shown that when a timeout < 1000ms is specified the IcmpSendEcho calls fail (apparently) randomly. Once the timeout is 1000ms or greater it works as expected. Therefore I've updated the fix to use 1000ms as a minimum. The existing logic ensures that the ttl is less than the specified timeout in any case: http://cr.openjdk.java.net/~robm/8143397/webrev.02/ -Rob On 01/12/15 14:59, Rob McKenna wrote: It appears that there is an undocumented minimum timeout in the IcmpSendEcho* functions. If the timeout parameter is set to a number below this minimum timeout it is effectively ignored. Thus if you wanted to ensure that you could ping a particular host within a certain timeout less than the undocumented minimum, you could potentially receive a false positive. (i.e. if you set the timeout to 20ms but the ping takes 30ms, IcmpSendEcho will still succeed) The following fix checks the icmp reply packet and compares the round trip time to the requested timeout parameter before deciding whether the call was successful or not: http://cr.openjdk.java.net/~robm/8143397/webrev.01/ -Rob
