Skip to content

Commit

Permalink
Revert getaddrinfo_a()
Browse files Browse the repository at this point in the history 
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]
  • Loading branch information
@mmasaki
mmasaki committed Dec 7, 2020
1 parent 1ba05f5 commit 5d8bcc4
Show file tree
Hide file tree
Showing 9 changed files with 0 additions and 289 deletions.
2 changes: 0 additions & 2 deletions configure.ac
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 0 additions & 2 deletions ext/socket/extconf.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,6 @@ def test_recvmsg_with_msg_peek_creates_fds(headers)
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)
Expand Down Expand Up @@ -506,7 +505,6 @@ def test_recvmsg_with_msg_peek_creates_fds(headers)
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
Expand Down
7 changes: 0 additions & 7 deletions ext/socket/ipsocket.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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


/*
Expand Down
248 changes: 0 additions & 248 deletions ext/socket/raddrinfo.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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)
{
Expand All @@ -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)
{
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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
}
4 changes: 0 additions & 4 deletions ext/socket/rubysocket.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 0 additions & 4 deletions ext/socket/socket.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 0 additions & 2 deletions include/ruby/internal/intern/process.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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*);
Expand Down
10 changes: 0 additions & 10 deletions process.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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, ... */
Expand Down Expand Up @@ -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();
}

Expand Down
Loading

0 comments on commit 5d8bcc4

Please sign in to comment.