diff --git a/ChangeLog b/ChangeLog index dc0c1aed39..7c0b93dedd 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +Thu Sep 18 05:44:05 2014 Eric Wong + + * ext/socket/init.c (rsock_connect): refactor for blocking + (wait_connectable): clear error before wait + [Bug #9356] + Wed Sep 17 23:12:36 2014 NARUSE, Yui * lib/uri/rfc3986_parser.rb: specify a regexp for :OPAQUE; generic.rb diff --git a/ext/socket/init.c b/ext/socket/init.c index f6cd2bce7b..670349ae14 100644 --- a/ext/socket/init.c +++ b/ext/socket/init.c @@ -339,68 +339,65 @@ rsock_socket(int domain, int type, int proto) return fd; } +/* emulate blocking connect behavior on EINTR or non-blocking socket */ static int wait_connectable(int fd) { - int sockerr; + int sockerr, revents; socklen_t sockerrlen; - int revents; - int ret; - for (;;) { - /* - * Stevens book says, successful finish turn on RB_WAITFD_OUT and - * failure finish turn on both RB_WAITFD_IN and RB_WAITFD_OUT. - */ - revents = rb_wait_for_single_fd(fd, RB_WAITFD_IN|RB_WAITFD_OUT, NULL); + /* only to clear pending error */ + sockerrlen = (socklen_t)sizeof(sockerr); + if (getsockopt(fd, SOL_SOCKET, SO_ERROR, (void *)&sockerr, &sockerrlen) < 0) + return -1; - if (revents & (RB_WAITFD_IN|RB_WAITFD_OUT)) { - sockerrlen = (socklen_t)sizeof(sockerr); - ret = getsockopt(fd, SOL_SOCKET, SO_ERROR, (void *)&sockerr, &sockerrlen); + /* + * Stevens book says, successful finish turn on RB_WAITFD_OUT and + * failure finish turn on both RB_WAITFD_IN and RB_WAITFD_OUT. + * So it's enough to wait only RB_WAITFD_OUT and check the pending error + * by getsockopt(). + * + * Note: rb_wait_for_single_fd already retries on EINTR/ERESTART + */ + revents = rb_wait_for_single_fd(fd, RB_WAITFD_IN|RB_WAITFD_OUT, NULL); - /* - * Solaris getsockopt(SO_ERROR) return -1 and set errno - * in getsockopt(). Let's return immediately. - */ - if (ret < 0) - break; - if (sockerr == 0) { - if (revents & RB_WAITFD_OUT) - break; - else - continue; /* workaround for winsock */ - } + if (revents < 0) + return -1; - /* BSD and Linux use sockerr. */ - errno = sockerr; - ret = -1; - break; - } + sockerrlen = (socklen_t)sizeof(sockerr); + if (getsockopt(fd, SOL_SOCKET, SO_ERROR, (void *)&sockerr, &sockerrlen) < 0) + return -1; - if ((revents & (RB_WAITFD_IN|RB_WAITFD_OUT)) == RB_WAITFD_OUT) { - ret = 0; - break; - } + switch (sockerr) { + case 0: + /* + * be defensive in case some platforms set SO_ERROR on the original, + * interrupted connect() + */ + case EINTR: +#ifdef ERESTART + case ERESTART: +#endif + case EAGAIN: +#ifdef EINPROGRESS + case EINPROGRESS: +#endif +#ifdef EALREADY + case EALREADY: +#endif +#ifdef EISCONN + case EISCONN: +#endif + return 0; /* success */ + default: + /* likely (but not limited to): ECONNREFUSED, ETIMEDOUT, EHOSTUNREACH */ + errno = sockerr; + return -1; } - return ret; + return 0; } -#ifdef __CYGWIN__ -#define WAIT_IN_PROGRESS 10 -#endif -#ifdef __APPLE__ -#define WAIT_IN_PROGRESS 10 -#endif -#ifdef __linux__ -/* returns correct error */ -#define WAIT_IN_PROGRESS 0 -#endif -#ifndef WAIT_IN_PROGRESS -/* BSD origin code apparently has a problem */ -#define WAIT_IN_PROGRESS 1 -#endif - struct connect_arg { int fd; const struct sockaddr *sockaddr; @@ -429,11 +426,6 @@ rsock_connect(int fd, const struct sockaddr *sockaddr, int len, int socks) int status; rb_blocking_function_t *func = connect_blocking; struct connect_arg arg; -#if WAIT_IN_PROGRESS > 0 - int wait_in_progress = -1; - int sockerr; - socklen_t sockerrlen; -#endif arg.fd = fd; arg.sockaddr = sockaddr; @@ -441,76 +433,22 @@ rsock_connect(int fd, const struct sockaddr *sockaddr, int len, int socks) #if defined(SOCKS) && !defined(SOCKS5) if (socks) func = socks_connect_blocking; #endif - for (;;) { - status = (int)BLOCKING_REGION_FD(func, &arg); - if (status < 0) { - switch (errno) { - case EINTR: -#if defined(ERESTART) - case ERESTART: -#endif - continue; + status = (int)BLOCKING_REGION_FD(func, &arg); - case EAGAIN: + if (status < 0) { + switch (errno) { + case EINTR: +#ifdef ERESTART + case ERESTART: +#endif + case EAGAIN: #ifdef EINPROGRESS - case EINPROGRESS: + case EINPROGRESS: #endif -#if WAIT_IN_PROGRESS > 0 - sockerrlen = (socklen_t)sizeof(sockerr); - status = getsockopt(fd, SOL_SOCKET, SO_ERROR, (void *)&sockerr, &sockerrlen); - if (status) break; - if (sockerr) { - status = -1; - errno = sockerr; - break; - } -#endif -#ifdef EALREADY - case EALREADY: -#endif -#if WAIT_IN_PROGRESS > 0 - wait_in_progress = WAIT_IN_PROGRESS; -#endif - status = wait_connectable(fd); - if (status) { - break; - } - errno = 0; - continue; - -#if WAIT_IN_PROGRESS > 0 - case EINVAL: - if (wait_in_progress-- > 0) { - /* - * connect() after EINPROGRESS returns EINVAL on - * some platforms, need to check true error - * status. - */ - sockerrlen = (socklen_t)sizeof(sockerr); - status = getsockopt(fd, SOL_SOCKET, SO_ERROR, (void *)&sockerr, &sockerrlen); - if (!status && !sockerr) { - struct timeval tv = {0, 100000}; - rb_thread_wait_for(tv); - continue; - } - status = -1; - errno = sockerr; - } - break; -#endif - -#ifdef EISCONN - case EISCONN: - status = 0; - errno = 0; - break; -#endif - default: - break; - } - } - return status; + return wait_connectable(fd); + } } + return status; } static void