Feature #15553: Addrinfo.getaddrinfo supports timeout

• Status: Closed
• Assignee: Glass_saga (Masaki Matsushita)
• Target version: 2.7

[Glass_saga (Masaki Matsushita)]

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.

[Glass_saga (Masaki Matsushita)]

• Related to Feature #14430: net/http: use Socket.tcp with connect_timeout, instead of TCPSocket.open wrapped in Timeout.timeout added

[ahorek (Pavel Rosický)]

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?

[naruse (Yui NARUSE)]

Why hrtime.h is included?

[Glass_saga (Masaki Matsushita)]

Why hrtime.h is included?

It's unnecessary. I'll remove it.

[akr (Akira Tanaka)]

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.

[akr (Akira Tanaka)]

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?

[Eregon (Benoit Daloze)]

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).

[Glass_saga (Masaki Matsushita)]

• 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.

[maciej.mensfeld (Maciej Mensfeld)]

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.

[Glass_saga (Masaki Matsushita)]

• Related to Bug #14997: Socket connect timeout exceeds the timeout value for added

[shyouhei (Shyouhei Urabe)]

Reviewed the patch and found no issues. So LGTM.

[Glass_saga (Masaki Matsushita)]

Thank you for reviewing. I will commit it.

[Glass_saga (Masaki Matsushita)]

• Status changed from Open to Closed
• Assignee set to Glass_saga (Masaki Matsushita)

committed in 6382f5cc91ac9e36776bc854632d9a1237250da7

[Glass_saga (Masaki Matsushita)]

• Related to Feature #16381: Accept resolv_timeout in Net::HTTP added