From 5d8bcc4870601ab1ee05346346f241d4a805aac9 Mon Sep 17 00:00:00 2001 From: Masaki Matsushita Date: Mon, 7 Dec 2020 12:49:44 +0900 Subject: [PATCH] Revert getaddrinfo_a() getaddrinfo_a() gets stuck after fork(). To avoid this, we need 1 second sleep to wait for internal worker threads of getaddrinfo_a() to be finished, but that is unacceptable. [Bug #17220] [Feature #17134] [Feature #17187] --- configure.ac | 2 - ext/socket/extconf.rb | 2 - ext/socket/ipsocket.c | 7 - ext/socket/raddrinfo.c | 248 ------------------------- ext/socket/rubysocket.h | 4 - ext/socket/socket.c | 4 - include/ruby/internal/intern/process.h | 2 - process.c | 10 - test/socket/test_socket.rb | 10 - 9 files changed, 289 deletions(-) diff --git a/configure.ac b/configure.ac index 84680cb29e..6c6d73f11f 100644 --- a/configure.ac +++ b/configure.ac @@ -1152,7 +1152,6 @@ AC_CHECK_LIB(crypt, crypt) # glibc (GNU/Linux, GNU/Hurd, GNU/kFreeBSD) AC_CHECK_LIB(dl, dlopen) # Dynamic linking for SunOS/Solaris and SYSV AC_CHECK_LIB(dld, shl_load) # Dynamic linking for HP-UX AC_CHECK_LIB(socket, shutdown) # SunOS/Solaris -AC_CHECK_LIB(anl, getaddrinfo_a) dnl Checks for header files. AC_HEADER_DIRENT @@ -1944,7 +1943,6 @@ AC_CHECK_FUNCS(fsync) AC_CHECK_FUNCS(ftruncate) AC_CHECK_FUNCS(ftruncate64) # used for Win32 platform AC_CHECK_FUNCS(getattrlist) -AC_CHECK_FUNCS(getaddrinfo_a) AC_CHECK_FUNCS(getcwd) AC_CHECK_FUNCS(getgidx) AC_CHECK_FUNCS(getgrnam) diff --git a/ext/socket/extconf.rb b/ext/socket/extconf.rb index 1a67e0013e..c86cc8f8c0 100644 --- a/ext/socket/extconf.rb +++ b/ext/socket/extconf.rb @@ -444,7 +444,6 @@ else test_func = "socket(0,0,0)" have_library("nsl", 't_open("", 0, (struct t_info *)NULL)', headers) # SunOS have_library("socket", "socket(0,0,0)", headers) # SunOS - have_library("anl", 'getaddrinfo_a', headers) end if have_func(test_func, headers) @@ -506,7 +505,6 @@ EOF unless have_func("gethostname((char *)0, 0)", headers) have_func("uname((struct utsname *)NULL)", headers) end - have_func("getaddrinfo_a", headers) ipv6 = false default_ipv6 = /haiku/ !~ RUBY_PLATFORM diff --git a/ext/socket/ipsocket.c b/ext/socket/ipsocket.c index 96782c76e5..646bc7c758 100644 --- a/ext/socket/ipsocket.c +++ b/ext/socket/ipsocket.c @@ -51,16 +51,9 @@ init_inetsock_internal(VALUE v) int family = AF_UNSPEC; const char *syscall = 0; -#ifdef HAVE_GETADDRINFO_A - arg->remote.res = rsock_addrinfo_a(arg->remote.host, arg->remote.serv, - family, SOCK_STREAM, - (type == INET_SERVER) ? AI_PASSIVE : 0, - arg->resolv_timeout); -#else arg->remote.res = rsock_addrinfo(arg->remote.host, arg->remote.serv, family, SOCK_STREAM, (type == INET_SERVER) ? AI_PASSIVE : 0); -#endif /* diff --git a/ext/socket/raddrinfo.c b/ext/socket/raddrinfo.c index c0cc0f37ea..a3e7e380d0 100644 --- a/ext/socket/raddrinfo.c +++ b/ext/socket/raddrinfo.c @@ -199,27 +199,6 @@ nogvl_getaddrinfo(void *arg) } #endif -#ifdef HAVE_GETADDRINFO_A -struct gai_suspend_arg -{ - struct gaicb *req; - struct timespec *timeout; -}; - -static void * -nogvl_gai_suspend(void *arg) -{ - int ret; - struct gai_suspend_arg *ptr = arg; - struct gaicb const *wait_reqs[1]; - - wait_reqs[0] = ptr->req; - ret = gai_suspend(wait_reqs, 1, ptr->timeout); - - return (void *)(VALUE)ret; -} -#endif - static int numeric_getaddrinfo(const char *node, const char *service, const struct addrinfo *hints, @@ -339,175 +318,6 @@ rb_getaddrinfo(const char *node, const char *service, return ret; } -#ifdef HAVE_GETADDRINFO_A -struct gaicbs { - struct gaicbs *next; - struct gaicb *gaicb; -}; - -/* linked list to retain all outstanding and ongoing requests */ -static struct gaicbs *requests = NULL; - -static void -gaicbs_add(struct gaicb *req) -{ - struct gaicbs *request; - - if (!req) return; - request = (struct gaicbs *)xmalloc(sizeof(struct gaicbs)); - request->gaicb = req; - request->next = requests; - - requests = request; -} - -static void -gaicbs_remove(struct gaicb *req) -{ - struct gaicbs *request = requests; - struct gaicbs *prev = NULL; - - if (!req) return; - - while (request) { - if (request->gaicb == req) break; - prev = request; - request = request->next; - } - - if (request) { - if (prev) { - prev->next = request->next; - } else { - requests = request->next; - } - xfree(request); - } -} - -static void -gaicbs_cancel_all(void) -{ - struct gaicbs *request = requests; - struct gaicbs *tmp, *prev = NULL; - int ret; - - while (request) { - ret = gai_cancel(request->gaicb); - if (ret == EAI_NOTCANCELED) { - // continue to next request - prev = request; - request = request->next; - } else { - // remove the request from the list - if (prev) { - prev->next = request->next; - } else { - requests = request->next; - } - tmp = request; - request = request->next; - xfree(tmp); - } - } -} - -static void -gaicbs_wait_all(void) -{ - struct gaicbs *request = requests; - int size = 0; - - // count gaicbs - while (request) { - size++; - request = request->next; - } - - // create list to wait - const struct gaicb *reqs[size]; - request = requests; - for (int i=0; request; i++) { - reqs[i] = request->gaicb; - request = request->next; - } - - // wait requests - gai_suspend(reqs, size, NULL); // ignore result intentionally -} - -#define MILLISECOND_IN_NANOSECONDS 1000000 - -/* A mitigation for [Bug #17220]. - It cancels all outstanding requests and waits for ongoing requests. - Then, it waits internal worker threads in getaddrinfo_a(3) to be finished. */ -void -rb_getaddrinfo_a_before_fork(void) -{ - gaicbs_cancel_all(); - gaicbs_wait_all(); - - /* wait worker threads in getaddrinfo_a(3) to be finished. - they will finish after 1 second sleep. */ - struct timespec ts = {1, 500 * MILLISECOND_IN_NANOSECONDS}; - nanosleep(&ts, NULL); -} - -int -rb_getaddrinfo_a(const char *node, const char *service, - const struct addrinfo *hints, - struct rb_addrinfo **res, struct timespec *timeout) -{ - struct addrinfo *ai; - int ret; - int allocated_by_malloc = 0; - - ret = numeric_getaddrinfo(node, service, hints, &ai); - if (ret == 0) - allocated_by_malloc = 1; - else { - struct gai_suspend_arg arg; - struct gaicb *reqs[1]; - struct gaicb req; - - req.ar_name = node; - req.ar_service = service; - req.ar_request = hints; - - reqs[0] = &req; - ret = getaddrinfo_a(GAI_NOWAIT, reqs, 1, NULL); - if (ret) return ret; - gaicbs_add(&req); - - arg.req = &req; - arg.timeout = timeout; - - ret = (int)(VALUE)rb_thread_call_without_gvl(nogvl_gai_suspend, &arg, RUBY_UBF_IO, 0); - gaicbs_remove(&req); - 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; - } - } - - - ret = gai_error(reqs[0]); - ai = reqs[0]->ar_result; - } - - if (ret == 0) { - *res = (struct rb_addrinfo *)xmalloc(sizeof(struct rb_addrinfo)); - (*res)->allocated_by_malloc = allocated_by_malloc; - (*res)->ai = ai; - } - return ret; -} -#endif - void rb_freeaddrinfo(struct rb_addrinfo *ai) { @@ -716,42 +526,6 @@ rsock_getaddrinfo(VALUE host, VALUE port, struct addrinfo *hints, int socktype_h return res; } -#ifdef HAVE_GETADDRINFO_A -struct rb_addrinfo* -rsock_getaddrinfo_a(VALUE host, VALUE port, struct addrinfo *hints, int socktype_hack, VALUE timeout) -{ - struct rb_addrinfo* res = NULL; - char *hostp, *portp; - int error; - char hbuf[NI_MAXHOST], pbuf[NI_MAXSERV]; - int additional_flags = 0; - - hostp = host_str(host, hbuf, sizeof(hbuf), &additional_flags); - portp = port_str(port, pbuf, sizeof(pbuf), &additional_flags); - - if (socktype_hack && hints->ai_socktype == 0 && str_is_number(portp)) { - hints->ai_socktype = SOCK_DGRAM; - } - hints->ai_flags |= additional_flags; - - if (NIL_P(timeout)) { - 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); - } - - if (error) { - if (hostp && hostp[strlen(hostp)-1] == '\n') { - rb_raise(rb_eSocket, "newline at the end of hostname"); - } - rsock_raise_socket_error("getaddrinfo_a", error); - } - - return res; -} -#endif - int rsock_fd_family(int fd) { @@ -777,20 +551,6 @@ rsock_addrinfo(VALUE host, VALUE port, int family, int socktype, int flags) return rsock_getaddrinfo(host, port, &hints, 1); } -#ifdef HAVE_GETADDRINFO_A -struct rb_addrinfo* -rsock_addrinfo_a(VALUE host, VALUE port, int family, int socktype, int flags, VALUE timeout) -{ - struct addrinfo hints; - - MEMZERO(&hints, struct addrinfo, 1); - hints.ai_family = family; - hints.ai_socktype = socktype; - hints.ai_flags = flags; - return rsock_getaddrinfo_a(host, port, &hints, 1, timeout); -} -#endif - VALUE rsock_ipaddr(struct sockaddr *sockaddr, socklen_t sockaddrlen, int norevlookup) { @@ -1071,11 +831,7 @@ call_getaddrinfo(VALUE node, VALUE service, hints.ai_flags = NUM2INT(flags); } -#ifdef HAVE_GETADDRINFO_A - res = rsock_getaddrinfo_a(node, service, &hints, socktype_hack, timeout); -#else res = rsock_getaddrinfo(node, service, &hints, socktype_hack); -#endif if (res == NULL) rb_raise(rb_eSocket, "host not found"); @@ -2873,8 +2629,4 @@ rsock_init_addrinfo(void) rb_define_method(rb_cAddrinfo, "marshal_dump", addrinfo_mdump, 0); rb_define_method(rb_cAddrinfo, "marshal_load", addrinfo_mload, 1); - -#ifdef HAVE_GETADDRINFO_A - rb_socket_before_fork_func = rb_getaddrinfo_a_before_fork; -#endif } diff --git a/ext/socket/rubysocket.h b/ext/socket/rubysocket.h index 9724fcc403..b0e494f54e 100644 --- a/ext/socket/rubysocket.h +++ b/ext/socket/rubysocket.h @@ -320,10 +320,6 @@ int rb_getnameinfo(const struct sockaddr *sa, socklen_t salen, char *host, size_ 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_addrinfo_a(VALUE host, VALUE port, int family, int socktype, int flags, VALUE timeout); -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 dc6588f82f..96365fbf91 100644 --- a/ext/socket/socket.c +++ b/ext/socket/socket.c @@ -1185,11 +1185,7 @@ sock_s_getaddrinfo(int argc, VALUE *argv, VALUE _) norevlookup = rsock_do_not_reverse_lookup; } -#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/include/ruby/internal/intern/process.h b/include/ruby/internal/intern/process.h index fa920cd90a..2b1005a205 100644 --- a/include/ruby/internal/intern/process.h +++ b/include/ruby/internal/intern/process.h @@ -28,8 +28,6 @@ RBIMPL_SYMBOL_EXPORT_BEGIN() /* process.c */ -RUBY_EXTERN void (* rb_socket_before_fork_func)(); - void rb_last_status_set(int status, rb_pid_t pid); VALUE rb_last_status_get(void); int rb_proc_exec(const char*); diff --git a/process.c b/process.c index 0542aca2e4..8ad555f8f6 100644 --- a/process.c +++ b/process.c @@ -331,9 +331,6 @@ static ID id_CLOCK_BASED_CLOCK_PROCESS_CPUTIME_ID; static ID id_MACH_ABSOLUTE_TIME_BASED_CLOCK_MONOTONIC; #endif static ID id_hertz; -#ifdef HAVE_GETADDRINFO_A -void (* rb_socket_before_fork_func)() = NULL; -#endif /* execv and execl are async-signal-safe since SUSv4 (POSIX.1-2008, XPG7) */ #if defined(__sun) && !defined(_XPG7) /* Solaris 10, 9, ... */ @@ -1624,13 +1621,6 @@ after_exec(void) static void before_fork_ruby(void) { -#ifdef HAVE_GETADDRINFO_A - if (rb_socket_before_fork_func) { - /* A mitigation for [Bug #17220]. See ext/socket/raddrinfo.c */ - rb_socket_before_fork_func(); - } -#endif - before_exec(); } diff --git a/test/socket/test_socket.rb b/test/socket/test_socket.rb index f0b3f66945..b5be0c7cf3 100644 --- a/test/socket/test_socket.rb +++ b/test/socket/test_socket.rb @@ -102,16 +102,6 @@ class TestSocket < Test::Unit::TestCase assert_nothing_raised('[ruby-core:29427]'){ TCPServer.open('localhost', 0) {} } end - def test_getaddrinfo_after_fork - skip "fork not supported" unless Process.respond_to?(:fork) - assert_normal_exit(<<-"end;", '[ruby-core:100329] [Bug #17220]') - require "socket" - Socket.getaddrinfo("localhost", nil) - pid = fork { Socket.getaddrinfo("localhost", nil) } - assert_equal pid, Timeout.timeout(3) { Process.wait(pid) } - end; - end - def test_getnameinfo assert_raise(SocketError) { Socket.getnameinfo(["AF_UNIX", 80, "0.0.0.0"]) }