commit ebd4ab1506517b32ee3bd5bd1597b4373aa56ee7 Author: Peter Retzlaff peter.retzlaff@student.hpi.uni-potsdam.de Date: Mon May 27 19:16:43 2013 +0000

Prepare patch for ticket 5129 for merging.

- Preserve old eventdns code. - Add function to close sockets cross-platform, without accounting. - Add changes/ file. --- changes/ticket5129 | 3 ++ src/common/compat.c | 141 ++++++++++++++++++++++++++++++++++++++------------- src/common/compat.h | 12 +++++ src/ext/eventdns.c | 9 ++++ src/or/connection.c | 21 ++------ 5 files changed, 133 insertions(+), 53 deletions(-)

diff --git a/changes/ticket5129 b/changes/ticket5129 new file mode 100644 index 0000000..c05ca68 --- /dev/null +++ b/changes/ticket5129 @@ -0,0 +1,3 @@ + o Minor features: + - Use the SOCK_NONBLOCK socket type, if supported, to open nonblocking + sockets in a single system call. Implements ticket #5129. diff --git a/src/common/compat.c b/src/common/compat.c index 0e943f3..af8f288 100644 --- a/src/common/compat.c +++ b/src/common/compat.c @@ -948,24 +948,40 @@ socket_accounting_unlock(void) }

/** As close(), but guaranteed to work for sockets across platforms (including - * Windows, where close()ing a socket doesn't work. Returns 0 on success, -1 - * on failure. */ + * Windows, where close()ing a socket doesn't work. Returns 0 on success and + * the socket error code on failure. */ int -tor_close_socket(tor_socket_t s) +tor_close_socket_simple(tor_socket_t s) { int r = 0;

/* On Windows, you have to call close() on fds returned by open(), - * and closesocket() on fds returned by socket(). On Unix, everything - * gets close()'d. We abstract this difference by always using - * tor_close_socket to close sockets, and always using close() on - * files. - */ -#if defined(_WIN32) - r = closesocket(s); -#else - r = close(s); -#endif + * and closesocket() on fds returned by socket(). On Unix, everything + * gets close()'d. We abstract this difference by always using + * tor_close_socket to close sockets, and always using close() on + * files. + */ + #if defined(_WIN32) + r = closesocket(s); + #else + r = close(s); + #endif + + if (r != 0) { + int err = tor_socket_errno(-1); + log_info(LD_NET, "Close returned an error: %s", tor_socket_strerror(err)); + return err; + } + + return r; +} + +/** As tor_close_socket_simple(), but keeps track of the number + * of open sockets. Returns 0 on success, -1 on failure. */ +int +tor_close_socket(tor_socket_t s) +{ + int r = tor_close_socket_simple(s);

socket_accounting_lock(); #ifdef DEBUG_SOCKET_COUNTING @@ -980,13 +996,11 @@ tor_close_socket(tor_socket_t s) if (r == 0) { --n_sockets_open; } else { - int err = tor_socket_errno(-1); - log_info(LD_NET, "Close returned an error: %s", tor_socket_strerror(err)); #ifdef _WIN32 - if (err != WSAENOTSOCK) + if (r != WSAENOTSOCK) --n_sockets_open; #else - if (err != EBADF) + if (r != EBADF) --n_sockets_open; #endif r = -1; @@ -1032,17 +1046,38 @@ mark_socket_open(tor_socket_t s) tor_socket_t tor_open_socket(int domain, int type, int protocol) { + return tor_open_socket_with_extensions(domain, type, protocol, 1, 0); +} + +/** As socket(), but creates a nonblocking socket and + * counts the number of open sockets. */ +tor_socket_t +tor_open_socket_nonblocking(int domain, int type, int protocol) +{ + return tor_open_socket_with_extensions(domain, type, protocol, 1, 1); +} + +/** As socket(), but counts the number of open sockets and handles + * socket creation with either of SOCK_CLOEXEC and SOCK_NONBLOCK specified. + * <b>cloexec</b> and <b>nonblock</b> should be either 0 or 1 to indicate + * if the corresponding extension should be used.*/ +tor_socket_t +tor_open_socket_with_extensions(int domain, int type, int protocol, + int cloexec, int nonblock) +{ tor_socket_t s; -#ifdef SOCK_CLOEXEC - s = socket(domain, type|SOCK_CLOEXEC, protocol); +#if defined(SOCK_CLOEXEC) && defined(SOCK_NONBLOCK) + int ext_flags = (cloexec ? SOCK_CLOEXEC : 0) | + (nonblock ? SOCK_NONBLOCK : 0); + s = socket(domain, type|ext_flags, protocol); if (SOCKET_OK(s)) goto socket_ok; /* If we got an error, see if it is EINVAL. EINVAL might indicate that, - * even though we were built on a system with SOCK_CLOEXEC support, we - * are running on one without. */ + * even though we were built on a system with SOCK_CLOEXEC and SOCK_NONBLOCK + * support, we are running on one without. */ if (errno != EINVAL) return s; -#endif /* SOCK_CLOEXEC */ +#endif /* SOCK_CLOEXEC && SOCK_NONBLOCK */

s = socket(domain, type, protocol); if (! SOCKET_OK(s)) @@ -1051,15 +1086,18 @@ tor_open_socket(int domain, int type, int protocol) #if defined(FD_CLOEXEC) if (fcntl(s, F_SETFD, FD_CLOEXEC) == -1) { log_warn(LD_FS,"Couldn't set FD_CLOEXEC: %s", strerror(errno)); -#if defined(_WIN32) - closesocket(s); -#else - close(s); -#endif - return -1; + tor_close_socket_simple(s); + return TOR_INVALID_SOCKET; } #endif

+ if (nonblock) { + if (set_socket_nonblocking(s) == -1) { + tor_close_socket_simple(s); + return TOR_INVALID_SOCKET; + } + } + goto socket_ok; /* So that socket_ok will not be unused. */

socket_ok: @@ -1070,19 +1108,41 @@ tor_open_socket(int domain, int type, int protocol) return s; }

-/** As socket(), but counts the number of open sockets. */ +/** As accept(), but counts the number of open sockets. */ tor_socket_t tor_accept_socket(tor_socket_t sockfd, struct sockaddr *addr, socklen_t *len) { + return tor_accept_socket_with_extensions(sockfd, addr, len, 1, 0); +} + +/** As accept(), but returns a nonblocking socket and + * counts the number of open sockets. */ +tor_socket_t +tor_accept_socket_nonblocking(tor_socket_t sockfd, struct sockaddr *addr, + socklen_t *len) +{ + return tor_accept_socket_with_extensions(sockfd, addr, len, 1, 1); +} + +/** As accept(), but counts the number of open sockets and handles + * socket creation with either of SOCK_CLOEXEC and SOCK_NONBLOCK specified. + * <b>cloexec</b> and <b>nonblock</b> should be either 0 or 1 to indicate + * if the corresponding extension should be used.*/ +tor_socket_t +tor_accept_socket_with_extensions(tor_socket_t sockfd, struct sockaddr *addr, + socklen_t *len, int cloexec, int nonblock) +{ tor_socket_t s; -#if defined(HAVE_ACCEPT4) && defined(SOCK_CLOEXEC) - s = accept4(sockfd, addr, len, SOCK_CLOEXEC); +#if defined(HAVE_ACCEPT4) && defined(SOCK_CLOEXEC) && defined(SOCK_NONBLOCK) + int ext_flags = (cloexec ? SOCK_CLOEXEC : 0) | + (nonblock ? SOCK_NONBLOCK : 0); + s = accept4(sockfd, addr, len, ext_flags); if (SOCKET_OK(s)) goto socket_ok; /* If we got an error, see if it is ENOSYS. ENOSYS indicates that, * even though we were built on a system with accept4 support, we * are running on one without. Also, check for EINVAL, which indicates that - * we are missing SOCK_CLOEXEC support. */ + * we are missing SOCK_CLOEXEC/SOCK_NONBLOCK support. */ if (errno != EINVAL && errno != ENOSYS) return s; #endif @@ -1092,13 +1152,22 @@ tor_accept_socket(tor_socket_t sockfd, struct sockaddr *addr, socklen_t *len) return s;

#if defined(FD_CLOEXEC) - if (fcntl(s, F_SETFD, FD_CLOEXEC) == -1) { - log_warn(LD_NET, "Couldn't set FD_CLOEXEC: %s", strerror(errno)); - close(s); - return TOR_INVALID_SOCKET; + if (cloexec) { + if (fcntl(s, F_SETFD, FD_CLOEXEC) == -1) { + log_warn(LD_NET, "Couldn't set FD_CLOEXEC: %s", strerror(errno)); + tor_close_socket_simple(s); + return TOR_INVALID_SOCKET; + } } #endif

+ if (nonblock) { + if (set_socket_nonblocking(s) == -1) { + tor_close_socket_simple(s); + return TOR_INVALID_SOCKET; + } + } + goto socket_ok; /* So that socket_ok will not be unused. */

socket_ok: diff --git a/src/common/compat.h b/src/common/compat.h index 258fc99..89a5d7f 100644 --- a/src/common/compat.h +++ b/src/common/compat.h @@ -450,10 +450,22 @@ typedef int socklen_t; #define TOR_INVALID_SOCKET (-1) #endif

+int tor_close_socket_simple(tor_socket_t s); int tor_close_socket(tor_socket_t s); +tor_socket_t tor_open_socket_with_extensions( + int domain, int type, int protocol, + int cloexec, int nonblock); tor_socket_t tor_open_socket(int domain, int type, int protocol); +tor_socket_t tor_open_socket_nonblocking(int domain, int type, int protocol); tor_socket_t tor_accept_socket(tor_socket_t sockfd, struct sockaddr *addr, socklen_t *len); +tor_socket_t tor_accept_socket_nonblocking(tor_socket_t sockfd, + struct sockaddr *addr, + socklen_t *len); +tor_socket_t tor_accept_socket_with_extensions(tor_socket_t sockfd, + struct sockaddr *addr, + socklen_t *len, + int cloexec, int nonblock); int get_n_open_sockets(void);

#define tor_socket_send(s, buf, len, flags) send(s, buf, len, flags) diff --git a/src/ext/eventdns.c b/src/ext/eventdns.c index 66280cc..8b934c4 100644 --- a/src/ext/eventdns.c +++ b/src/ext/eventdns.c @@ -2298,6 +2298,10 @@ _evdns_nameserver_add_impl(const struct sockaddr *address,

evtimer_set(&ns->timeout_event, nameserver_prod_callback, ns);

+#if 1 + ns->socket = tor_open_socket_nonblocking(address->sa_family, SOCK_DGRAM, 0); + if (!SOCKET_OK(ns->socket)) { err = 1; goto out1; } +#else ns->socket = tor_open_socket(address->sa_family, SOCK_DGRAM, 0); if (ns->socket < 0) { err = 1; goto out1; } #ifdef _WIN32 @@ -2314,6 +2318,7 @@ _evdns_nameserver_add_impl(const struct sockaddr *address, } #endif

+#endif /* 1 */ if (global_bind_addr_is_set && !sockaddr_is_loopback((struct sockaddr*)&global_bind_address)) { if (bind(ns->socket, (struct sockaddr *)&global_bind_address, @@ -3473,8 +3478,12 @@ main(int c, char **v) { if (servertest) { int sock; struct sockaddr_in my_addr; +#if 1 + sock = tor_open_socket_nonblocking(PF_INET, SOCK_DGRAM, 0) +#else sock = tor_open_socket(PF_INET, SOCK_DGRAM, 0); fcntl(sock, F_SETFL, O_NONBLOCK); +#endif my_addr.sin_family = AF_INET; my_addr.sin_port = htons(10053); my_addr.sin_addr.s_addr = INADDR_ANY; diff --git a/src/or/connection.c b/src/or/connection.c index 6a3cc7b..28de35b 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -970,7 +970,7 @@ connection_listener_new(const struct sockaddr *listensockaddr, log_notice(LD_NET, "Opening %s on %s", conn_type_to_string(type), fmt_addrport(&addr, usePort));

- s = tor_open_socket(tor_addr_family(&addr), + s = tor_open_socket_nonblocking(tor_addr_family(&addr), is_tcp ? SOCK_STREAM : SOCK_DGRAM, is_tcp ? IPPROTO_TCP: IPPROTO_UDP); if (!SOCKET_OK(s)) { @@ -1054,7 +1054,7 @@ connection_listener_new(const struct sockaddr *listensockaddr, strerror(errno)); goto err; } - s = tor_open_socket(AF_UNIX, SOCK_STREAM, 0); + s = tor_open_socket_nonblocking(AF_UNIX, SOCK_STREAM, 0); if (! SOCKET_OK(s)) { log_warn(LD_NET,"Socket creation failed: %s.", strerror(errno)); goto err; @@ -1102,9 +1102,6 @@ connection_listener_new(const struct sockaddr *listensockaddr, tor_assert(0); }

- if (set_socket_nonblocking(s) == -1) - goto err; - lis_conn = listener_connection_new(type, listensockaddr->sa_family); conn = TO_CONN(lis_conn); conn->socket_family = listensockaddr->sa_family; @@ -1252,7 +1249,7 @@ connection_handle_listener_read(connection_t *conn, int new_type) tor_assert((size_t)remotelen >= sizeof(struct sockaddr_in)); memset(&addrbuf, 0, sizeof(addrbuf));

- news = tor_accept_socket(conn->s,remote,&remotelen); + news = tor_accept_socket_nonblocking(conn->s,remote,&remotelen); if (!SOCKET_OK(news)) { /* accept() error */ int e = tor_socket_errno(conn->s); if (ERRNO_IS_ACCEPT_EAGAIN(e)) { @@ -1272,10 +1269,6 @@ connection_handle_listener_read(connection_t *conn, int new_type) (int)news,(int)conn->s);

make_socket_reuseable(news); - if (set_socket_nonblocking(news) == -1) { - tor_close_socket(news); - return 0; - }

if (options->ConstrainedSockets) set_constrained_socket_buffers(news, (int)options->ConstrainedSockSize); @@ -1467,7 +1460,7 @@ connection_connect(connection_t *conn, const char *address, return -1; }

- s = tor_open_socket(protocol_family,SOCK_STREAM,IPPROTO_TCP); + s = tor_open_socket_nonblocking(protocol_family,SOCK_STREAM,IPPROTO_TCP); if (! SOCKET_OK(s)) { *socket_error = tor_socket_errno(-1); log_warn(LD_NET,"Error creating network socket: %s", @@ -1509,12 +1502,6 @@ connection_connect(connection_t *conn, const char *address, } }

- if (set_socket_nonblocking(s) == -1) { - *socket_error = tor_socket_errno(s); - tor_close_socket(s); - return -1; - } - if (options->ConstrainedSockets) set_constrained_socket_buffers(s, (int)options->ConstrainedSockSize);