Project

General

Profile

Feature #15553

Addrinfo.getaddrinfo supports timeout

Added by Glass_saga (Masaki Matsushita) 8 months ago. Updated 14 days ago.

Status:
Closed
Priority:
Normal
Target version:
[ruby-core:91200]

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

patch.diff (13.2 KB) patch.diff Glass_saga (Masaki Matsushita), 01/21/2019 03:58 AM
patch2.diff (11.6 KB) patch2.diff get rid of Timeout from ext/socket/lib/socket.rb. If getaddrinfo_a() is not available, timeout is ignored. Glass_saga (Masaki Matsushita), 03/24/2019 05:39 AM

Related issues

Related to Ruby master - Feature #14430: net/http: use Socket.tcp with connect_timeout, instead of TCPSocket.open wrapped in Timeout.timeoutOpenActions
Related to Ruby master - Bug #14997: Socket connect timeout exceeds the timeout value for OpenActions

History

#1

Updated by Glass_saga (Masaki Matsushita) 8 months 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√Ĺ) 8 months 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) 8 months ago

Why hrtime.h is included?

Updated by Glass_saga (Masaki Matsushita) 8 months ago

Why hrtime.h is included?

It's unnecessary. I'll remove it.

Updated by akr (Akira Tanaka) 8 months 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) 8 months 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) 8 months 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) 6 months ago

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

#10

Updated by Glass_saga (Masaki Matsushita) about 2 months ago

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

Updated by shyouhei (Shyouhei Urabe) 24 days ago

Reviewed the patch and found no issues. So LGTM.

Updated by Glass_saga (Masaki Matsushita) 24 days ago

Thank you for reviewing. I will commit it.

Updated by Glass_saga (Masaki Matsushita) 14 days ago

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

committed in 6382f5cc91ac9e36776bc854632d9a1237250da7

Also available in: Atom PDF