r12759@Kushana: nickm | 2007-04-20 08:47:20 -0400

 Track the number of connection_t separately from the number of open sockets.  It is already possible to have connections that do not count: resolving conns, for one.  Once we move from socketpairs to linked conns, and once we do dns proxying, there will be lots of such connections.


svn:r9994

ChangeLog

@@ -17,6 +17,11 @@ Changes in version 0.2.0.1-alpha - 2007-??-??
       in the network. Implements proposal 107, suggested by Kevin Bauer
       and Damon McCoy.
 
+  o Minor fixes (resource management):
+    - Count the number of open sockets separately from the number of active
+      connection_t objects.  This will let us avoid underusing our
+      allocated connection limit.
+
   o Minor features (build):
     - Make autoconf search for libevent, openssl, and zlib consistently.
     - Update deprecated macros in configure.in

doc/TODO

@@ -100,8 +100,22 @@ Things we'd like to do in 0.2.0.x:
       "who can change what" in local_routerstatus explicit.  Make
       local_routerstatus (or equivalent) subsume all places to go for "what
       router is this?"
-    - Remove socketpair-based bridges conns, and the word "bridge".  (Use
+    . Remove socketpair-based bridges conns, and the word "bridge".  (Use
       shared (or connected) buffers for communication, rather than sockets.)
+      o Design
+        o Pick a term.  The term is now "linked connection."
+        o Figure out how to ensure that handle_read is always called.
+          (Use event_active; keep active events in a list; use event_once
+          to make sure that we call the event base dispatch function enough.)
+      . Implement
+        o Count connections and sockets separately
+        - Allow connections with s == -1
+        - Add a linked_conn field; it should get marked when we're marked.
+        - Add a function to move bytes from buffer to buffer.
+        - Have handle_read dtrt for linked connections
+        - Have an activate/deactivate_linked_connection function.
+        - Have activated functions added to a list on first activation, and
+          that list made active before calls to event_loop.
     - Generate torrc.{complete|sample}.in, tor.1.in, the HTML manual, and the
       online config documentation from a single source.
     - Have clients do TLS connection rotation less often than "every 10

src/common/compat.c

@@ -468,6 +468,48 @@ touch_file(const char *fname)
   return 0;
 }
 
+/** Count of number of sockets currently open.  (Undercounts sockets opened by
+ * eventdns and libevent.) */
+static int n_sockets_open = 0;
+
+/** As close(), but guaranteed to work for sockets across platforms (including
+ * Windows, where close()ing a socket doesn't work. */
+void
+tor_close_socket(int s)
+{
+  /* 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.
+   */
+#ifdef USE_BSOCKETS
+  bclose(s);
+#elif defined(MS_WINDOWS)
+  closesocket(s);
+#else
+  close(s);
+#endif
+  --n_sockets_open;
+}
+
+/** As socket(), but counts the number of open sockets. */
+int
+tor_open_socket(int domain, int type, int protocol)
+{
+  int s = socket(domain, type, protocol);
+  if (s >= 0)
+    ++n_sockets_open;
+  return s;
+}
+
+/** Return the number of sockets we currently have opened. */
+int
+get_n_open_sockets(void)
+{
+  return n_sockets_open;
+}
+
 /** Turn <b>socket</b> into a nonblocking socket.
  */
 void
@@ -537,7 +579,7 @@ tor_socketpair(int family, int type, int protocol, int fd[2])
       return -EINVAL;
     }
 
-    listener = socket(AF_INET, type, 0);
+    listener = tor_open_socket(AF_INET, type, 0);
     if (listener < 0)
       return -tor_socket_errno(-1);
     memset(&listen_addr, 0, sizeof(listen_addr));
@@ -550,7 +592,7 @@ tor_socketpair(int family, int type, int protocol, int fd[2])
     if (listen(listener, 1) == -1)
       goto tidy_up_and_fail;
 
-    connector = socket(AF_INET, type, 0);
+    connector = tor_open_socket(AF_INET, type, 0);
     if (connector < 0)
       goto tidy_up_and_fail;
     /* We want to find out the port number to connect to.  */

src/common/compat.h

@@ -213,19 +213,10 @@ int touch_file(const char *fname);
 #endif
 
 /* ===== Net compatibility */
-#ifdef USE_BSOCKETS
-#define tor_close_socket(s) bclose(s)
-#elif defined(MS_WINDOWS)
-/** 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.
- */
-#define tor_close_socket(s) closesocket(s)
-#else
-#define tor_close_socket(s) close(s)
-#endif
+
+void tor_close_socket(int s);
+int tor_open_socket(int domain, int type, int protocol);
+int get_n_open_sockets(void);
 
 #ifdef USE_BSOCKETS
 #define tor_socket_send(s, buf, len, flags) bsend(s, buf, len, flags)

src/common/util.c

@@ -1914,7 +1914,7 @@ get_interface_address(int severity, uint32_t *addr)
   tor_assert(addr);
   *addr = 0;
 
-  sock = socket(PF_INET,SOCK_DGRAM,IPPROTO_UDP);
+  sock = tor_open_socket(PF_INET,SOCK_DGRAM,IPPROTO_UDP);
   if (sock < 0) {
     int e = tor_socket_errno(-1);
     log_fn(severity, LD_NET, "unable to create socket: %s",

src/or/connection.c

@@ -613,7 +613,16 @@ connection_create_listener(const char *listenaddress, uint16_t listenport,
   log_notice(LD_NET, "Opening %s on %s:%d",
              conn_type_to_string(type), address, usePort);
 
-  s = socket(PF_INET,SOCK_STREAM,IPPROTO_TCP);
+  if (get_n_open_sockets() >= get_options()->_ConnLimit-1) {
+    int n_conns = get_n_open_sockets();
+    log_warn(LD_NET,"Failing because we have %d connections already. Please "
+             "raise your ulimit -n.", n_conns);
+    control_event_general_status(LOG_WARN, "TOO_MANY_CONNECTIONS CURRENT=%d",
+                                 n_conns);
+    return NULL;
+  }
+
+  s = tor_open_socket(PF_INET,SOCK_STREAM,IPPROTO_TCP);
   if (s < 0) {
     log_warn(LD_NET,"Socket creation failed.");
     goto err;
@@ -853,7 +862,16 @@ connection_connect(connection_t *conn, const char *address,
   struct sockaddr_in dest_addr;
   or_options_t *options = get_options();
 
-  s = socket(PF_INET,SOCK_STREAM,IPPROTO_TCP);
+  if (get_n_open_sockets() >= get_options()->_ConnLimit-1) {
+    int n_conns = get_n_open_sockets();
+    log_warn(LD_NET,"Failing because we have %d connections already. Please "
+             "raise your ulimit -n.", n_conns);
+    control_event_general_status(LOG_WARN, "TOO_MANY_CONNECTIONS CURRENT=%d",
+                                 n_conns);
+    return -1;
+  }
+
+  s = tor_open_socket(PF_INET,SOCK_STREAM,IPPROTO_TCP);
   if (s < 0) {
     log_warn(LD_NET,"Error creating network socket: %s",
              tor_socket_strerror(tor_socket_errno(-1)));

src/or/main.c

@@ -68,6 +68,7 @@ static time_t time_of_last_signewnym = 0;
 static int signewnym_is_pending = 0;
 
 /** Array of all open connections.  The first n_conns elements are valid. */
+/*XXXX020 Should we just use a smartlist here? */
 static connection_t *connection_array[MAXCONNECTIONS+1] =
         { NULL };
 /** List of connections that have been marked for close and need to be freed
@@ -156,15 +157,12 @@ connection_add(connection_t *conn)
   tor_assert(conn);
   tor_assert(conn->s >= 0);
 
-  if (n_conns >= get_options()->_ConnLimit-1) {
-    log_warn(LD_NET,"Failing because we have %d connections already. Please "
-             "raise your ulimit -n.", n_conns);
-    control_event_general_status(LOG_WARN, "TOO_MANY_CONNECTIONS CURRENT=%d",
-                                 n_conns);
+  tor_assert(conn->conn_array_index == -1); /* can only connection_add once */
+  if (n_conns == MAXCONNECTIONS) {
+    log_warn(LD_BUG, "Unable to add a connection; MAXCONNECTIONS is set too "
+             "low.  This is a bug; tell the developers.");
     return -1;
   }
-
-  tor_assert(conn->conn_array_index == -1); /* can only connection_add once */
   conn->conn_array_index = n_conns;
   connection_array[n_conns] = conn;
 

src/tools/tor-resolve.c

@@ -153,7 +153,7 @@ do_resolve(const char *hostname, uint32_t sockshost, uint16_t socksport,
   *result_addr = 0;
   *result_hostname = NULL;
 
-  s = socket(PF_INET,SOCK_STREAM,IPPROTO_TCP);
+  s = tor_open_socket(PF_INET,SOCK_STREAM,IPPROTO_TCP);
   if (s<0) {
     log_sock_error("creating_socket", -1);
     return -1;