[tor-commits] [tor/master] Prepare patch for ticket 5129 for merging.

nickm at torproject.org nickm at torproject.org
Fri Aug 2 14:42:17 UTC 2013


commit ebd4ab1506517b32ee3bd5bd1597b4373aa56ee7
Author: Peter Retzlaff <peter.retzlaff at 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);
 





More information about the tor-commits mailing list