Feature #15553
closedAddrinfo.getaddrinfo supports timeout
Description
Currently, we use Timeout in Net::HTTP and other standard libraries.
lib/net/http.rb
945 s = Timeout.timeout(@open_timeout, Net::OpenTimeout) {
946 begin
947 TCPSocket.open(conn_address, conn_port, @local_host, @local_port)
948 rescue => e
949 raise e, "Failed to open TCP connection to " +
950 "#{conn_address}:#{conn_port} (#{e.message})"
951 end
952 }
Socket.tcp supports connect_timeout, but Addrinfo.getaddrinfo doesn't support timeout.
We need to use Timeout to wait name resolution.
In this patch, Addrinfo.getaddrinfo support timeout and Socket.tcp accepts resolv_timeout.
It uses getaddrinfo_a(3) if available, otherwise it uses Timeout.
We can avoid thread creation to make a TCP connection if getaddrinfo_a(3) is available.
Files
Updated by Glass_saga (Masaki Matsushita) almost 6 years ago
- Related to Feature #14430: net/http: use Socket.tcp with connect_timeout, instead of TCPSocket.open wrapped in Timeout.timeout added
Updated by ahorek (Pavel Rosický) almost 6 years ago
thanks for this PR. Many requests for fully async support in stdlib are blocked by this.
I think on Windows 8+ we can use https://docs.microsoft.com/cs-cz/windows/desktop/api/ws2tcpip/nf-ws2tcpip-getaddrinfoexa to avoid timeout?
Updated by naruse (Yui NARUSE) almost 6 years ago
Why hrtime.h is included?
Updated by Glass_saga (Masaki Matsushita) almost 6 years ago
Why hrtime.h is included?
It's unnecessary. I'll remove it.
Updated by akr (Akira Tanaka) over 5 years ago
I'm positive to use getaddrinfo_a if it is available.
However, if getaddrinfo_a is not available and timeout option is not set,
timeout library should not be loaded.
Also, it's welcome to improve resolv.rb.
Updated by akr (Akira Tanaka) over 5 years ago
I think timeout library is not effective for getaddrinfo method (without getaddrinfo_a).
What the patch tries to change on a platform which has no getaddrinfo_a?
Updated by Eregon (Benoit Daloze) over 5 years ago
akr (Akira Tanaka) wrote:
I think timeout library is not effective for getaddrinfo method (without getaddrinfo_a).
That was also my experience trying to use SIGVTALRM to interrupt getaddrinfo(3): it had no effect, the code ignores EINTR and only ends after 5 seconds (its default timeout I guess).
Updated by Glass_saga (Masaki Matsushita) over 5 years ago
- File patch2.diff patch2.diff added
I updated the patch.
- get rid of Timeout from ext/socket/lib/socket.rb. If getaddrinfo_a() is not available, timeout is ignored.
Updated by maciej.mensfeld (Maciej Mensfeld) over 5 years ago
This is also related to it: https://bugs.ruby-lang.org/issues/14997 - once the DNS timeout is in place (when available) I could give the #14997 a shot adding per ip timeout and including the resolving timeout in the overall timeout.
Updated by Glass_saga (Masaki Matsushita) over 5 years ago
- Related to Bug #14997: Socket connect timeout exceeds the timeout value for added
Updated by shyouhei (Shyouhei Urabe) about 5 years ago
Reviewed the patch and found no issues. So LGTM.
Updated by Glass_saga (Masaki Matsushita) about 5 years ago
Thank you for reviewing. I will commit it.
Updated by Glass_saga (Masaki Matsushita) about 5 years ago
- Status changed from Open to Closed
- Assignee set to Glass_saga (Masaki Matsushita)
committed in 6382f5cc91ac9e36776bc854632d9a1237250da7
Updated by Glass_saga (Masaki Matsushita) about 4 years ago
- Related to Feature #16381: Accept resolv_timeout in Net::HTTP added