https://git.ruby-lang.org/ruby.git/commit/?id=2038cc6cab

From 2038cc6cab6ceeffef3ec3a765c70ae684f829ed Mon Sep 17 00:00:00 2001
From: Kir Shatrov <shatrov@m...>
Date: Thu, 27 Aug 2020 08:39:13 +0100
Subject: Make Socket.getaddrinfo interruptible (#2827)

Before, Socket.getaddrinfo was using a blocking getaddrinfo(3) call.
That didn't allow to wrap it into Timeout.timeout or interrupt the thread in any way.

Combined with the default 10 sec resolv timeout on many Unix systems, this can
have a very noticeable effect on production Ruby apps being not
resilient to DNS outages and timing out name resolution, and being unable to fail fast even
with Timeout.timeout.

Since we already have support for getaddrinfo_a(3), the async version
of getaddrinfo, we should be able to make Socket.getaddrinfo leverage that
when getaddrinfo_a version is available in the system (hence #ifdef
HAVE_GETADDRINFO_A).

Related tickets:
https://bugs.ruby-lang.org/issues/16476
https://bugs.ruby-lang.org/issues/16381
https://bugs.ruby-lang.org/issues/14997

diff --git a/ext/socket/raddrinfo.c b/ext/socket/raddrinfo.c
index 079f66d..b601b1b 100644
--- a/ext/socket/raddrinfo.c
+++ b/ext/socket/raddrinfo.c
@@ -369,15 +369,16 @@ rb_getaddrinfo_a(const char *node, const char *service, https://github.com/ruby/ruby/blob/trunk/ext/socket/raddrinfo.c#L369
 	arg.timeout = timeout;
 
 	ret = (int)(VALUE)rb_thread_call_without_gvl(nogvl_gai_suspend, &arg, RUBY_UBF_IO, 0);
+    if (ret && ret != EAI_ALLDONE) {
+        /* EAI_ALLDONE indicates that the request already completed and gai_suspend was redundant */
+        /* on Ubuntu 18.04 (or other systems), gai_suspend(3) returns EAI_SYSTEM/ENOENT on timeout */
+        if (ret == EAI_SYSTEM && errno == ENOENT) {
+            return EAI_AGAIN;
+        } else {
+            return ret;
+        }
+    }
 
-	if (ret) {
-	    /* on Ubuntu 18.04 (or other systems), gai_suspend(3) returns EAI_SYSTEM/ENOENT on timeout */
-	    if (ret == EAI_SYSTEM && errno == ENOENT) {
-		return EAI_AGAIN;
-	    } else {
-		return ret;
-	    }
-	}
 
 	ret = gai_error(reqs[0]);
 	ai = reqs[0]->ar_result;
@@ -601,7 +602,7 @@ rsock_getaddrinfo(VALUE host, VALUE port, struct addrinfo *hints, int socktype_h https://github.com/ruby/ruby/blob/trunk/ext/socket/raddrinfo.c#L602
 }
 
 #ifdef HAVE_GETADDRINFO_A
-static struct rb_addrinfo*
+struct rb_addrinfo*
 rsock_getaddrinfo_a(VALUE host, VALUE port, struct addrinfo *hints, int socktype_hack, VALUE timeout)
 {
     struct rb_addrinfo* res = NULL;
@@ -619,10 +620,10 @@ rsock_getaddrinfo_a(VALUE host, VALUE port, struct addrinfo *hints, int socktype https://github.com/ruby/ruby/blob/trunk/ext/socket/raddrinfo.c#L620
     hints->ai_flags |= additional_flags;
 
     if (NIL_P(timeout)) {
-	error = rb_getaddrinfo(hostp, portp, hints, &res);
+        error = rb_getaddrinfo_a(hostp, portp, hints, &res, (struct timespec *)NULL);
     } else {
-	struct timespec _timeout = rb_time_timespec_interval(timeout);
-	error = rb_getaddrinfo_a(hostp, portp, hints, &res, &_timeout);
+        struct timespec _timeout = rb_time_timespec_interval(timeout);
+        error = rb_getaddrinfo_a(hostp, portp, hints, &res, &_timeout);
     }
 
     if (error) {
@@ -942,11 +943,7 @@ call_getaddrinfo(VALUE node, VALUE service, https://github.com/ruby/ruby/blob/trunk/ext/socket/raddrinfo.c#L943
     }
 
 #ifdef HAVE_GETADDRINFO_A
-    if (NIL_P(timeout)) {
-	res = rsock_getaddrinfo(node, service, &hints, socktype_hack);
-    } else {
 	res = rsock_getaddrinfo_a(node, service, &hints, socktype_hack, timeout);
-    }
 #else
     res = rsock_getaddrinfo(node, service, &hints, socktype_hack);
 #endif
diff --git a/ext/socket/rubysocket.h b/ext/socket/rubysocket.h
index fcea2a5..30b7a77 100644
--- a/ext/socket/rubysocket.h
+++ b/ext/socket/rubysocket.h
@@ -320,6 +320,10 @@ int rb_getnameinfo(const struct sockaddr *sa, socklen_t salen, char *host, size_ https://github.com/ruby/ruby/blob/trunk/ext/socket/rubysocket.h#L320
 int rsock_fd_family(int fd);
 struct rb_addrinfo *rsock_addrinfo(VALUE host, VALUE port, int family, int socktype, int flags);
 struct rb_addrinfo *rsock_getaddrinfo(VALUE host, VALUE port, struct addrinfo *hints, int socktype_hack);
+#ifdef HAVE_GETADDRINFO_A
+struct rb_addrinfo *rsock_getaddrinfo_a(VALUE host, VALUE port, struct addrinfo *hints, int socktype_hack, VALUE timeout);
+#endif
+
 VALUE rsock_fd_socket_addrinfo(int fd, struct sockaddr *addr, socklen_t len);
 VALUE rsock_io_socket_addrinfo(VALUE io, struct sockaddr *addr, socklen_t len);
 
diff --git a/ext/socket/socket.c b/ext/socket/socket.c
index e450462..3212467 100644
--- a/ext/socket/socket.c
+++ b/ext/socket/socket.c
@@ -1181,7 +1181,12 @@ sock_s_getaddrinfo(int argc, VALUE *argv, VALUE _) https://github.com/ruby/ruby/blob/trunk/ext/socket/socket.c#L1181
     if (NIL_P(revlookup) || !rsock_revlookup_flag(revlookup, &norevlookup)) {
 	norevlookup = rsock_do_not_reverse_lookup;
     }
-    res = rsock_getaddrinfo(host, port, &hints, 0);
+
+    #ifdef HAVE_GETADDRINFO_A
+        res = rsock_getaddrinfo_a(host, port, &hints, 0, Qnil);
+    #else
+        res = rsock_getaddrinfo(host, port, &hints, 0);
+    #endif
 
     ret = make_addrinfo(res, norevlookup);
     rb_freeaddrinfo(res);
diff --git a/test/socket/test_addrinfo.rb b/test/socket/test_addrinfo.rb
index d516c77..1421c15 100644
--- a/test/socket/test_addrinfo.rb
+++ b/test/socket/test_addrinfo.rb
@@ -103,7 +103,7 @@ class TestSocketAddrinfo < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/socket/test_addrinfo.rb#L103
   end
 
   def test_error_message
-    e = assert_raise_with_message(SocketError, /getaddrinfo:/) do
+    e = assert_raise_with_message(SocketError, /getaddrinfo/) do
       Addrinfo.ip("...")
     end
     m = e.message
-- 
cgit v0.10.2


--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/