Feature #17528
openMake Addrinfo.getaddrinfo fall back to Timeout.timeout for :resolv_timeout
Description
Currently, Addrinfo.getaddrinfo
ignores the :resolv_timeout
option if we are on a system without getaddrinfo_a
. It would be great if instead it would fall back to using Timeout.timeout
.
That way, we could get rid of a lot of the usage of Timeout.timeout
for systems that do have getaddrinfo_a
. For example, for Net::HTTP#connect we could easily then do something like this: https://github.com/ruby/ruby/compare/master...mohamedhafez:patch-3?diff=split.
The motivation for this is that the usage of Timeout.timeout is inherently unsafe, and it would be great to stop using it where we can (see https://www.mikeperham.com/2015/05/08/timeout-rubys-most-dangerous-api/ and http://blog.headius.com/2008/02/ruby-threadraise-threadkill-timeoutrb.html)
Updated by akr (Akira Tanaka) almost 4 years ago
I think Timeout.timeout doesn't work for Addrinfo.getaddrinfo
because Timeout.timeout cannot interrupt getaddrinfo function in C.
Updated by mohamedhafez (Mohamed Hafez) almost 4 years ago
According to https://bugs.ruby-lang.org/issues/12435, the only reason we are using Timeout.timeout
in Net::HTTP#connect
instead of using nonblocking io and IO.select
is so that we can place a timeout on the getaddrinfo function, @normalperson (Eric Wong) can you clarify here?
Updated by akr (Akira Tanaka) almost 4 years ago
How to test:
% vi /etc/resolv.conf # specify non-existing nameserver
% time ./ruby -rtimeout -rsocket -e 'Timeout.timeout(2) { Addrinfo.getaddrinfo("www.ruby-lang.org",nil) }'
-e:1:in `getaddrinfo': execution expired (Timeout::Error)
from -e:1:in `block in <main>'
from /home/akr/o0/lib/ruby/3.1.0/timeout.rb:112:in `timeout'
from -e:1:in `<main>'
./ruby -rtimeout -rsocket -e 0.14s user 0.00s system 0% cpu 20.136 total
This needs 20 second instead of 2 second.
Updated by mohamedhafez (Mohamed Hafez) almost 4 years ago
I was just doing a similar test actually, I think you're correct! I'll close this issue and re-submit a patch to Net::HTTP to replace Timeout::timeout
there, since it's not doing anything for DNS lookups anyway. Thank you!
Updated by hsbt (Hiroshi SHIBATA) almost 2 years ago
- Related to Feature #17525: Implement Happy Eyeballs Version 2 (RFC8305) in Socket.tcp added