Only connection_add connections once they have conn->s sett; refactor code around this. Should make stuff more bulletproof.

svn:r1788

src/or/circuit.c

@@ -619,7 +619,6 @@ void circuit_build_needed_circs(time_t now) {
   }
 
   /* XXX count idle rendezvous circs and build more */
-
 }
 
 /* update digest from the payload of cell. assign integrity part to cell. */

src/or/connection.c

@@ -365,10 +365,11 @@ static int connection_init_accepted_conn(connection_t *conn) {
   return 0;
 }
 
-/* take conn, make a nonblocking socket; try to connect to
+/* Take conn, make a nonblocking socket; try to connect to
  * addr:port (they arrive in *host order*). If fail, return -1. Else
  * assign s to conn->s: if connected return 1, if eagain return 0.
- * address is used to make the logs useful.
+ * address is used to make the logs useful.  On success, add 'conn' to
+ * the list of polled connections.
  */
 int connection_connect(connection_t *conn, char *address, uint32_t addr, uint16_t port) {
   int s;
@@ -398,6 +399,8 @@ int connection_connect(connection_t *conn, char *address, uint32_t addr, uint16_
     } else {
       /* it's in progress. set state appropriately and return. */
       conn->s = s;
+      if(connection_add(conn) < 0) /* no space, forget it */
+        return -1;
       log_fn(LOG_DEBUG,"connect in progress, socket %d.",s);
       return 0;
     }
@@ -406,6 +409,8 @@ int connection_connect(connection_t *conn, char *address, uint32_t addr, uint16_
   /* it succeeded. we're connected. */
   log_fn(LOG_INFO,"Connection to %s:%u established.",address,port);
   conn->s = s;
+  if(connection_add(conn) < 0) /* no space, forget it */
+    return -1;
   return 1;
 }
 

src/or/connection_edge.c

@@ -1179,11 +1179,6 @@ static int connection_exit_begin_conn(cell_t *cell, circuit_t *circ) {
   /* leave n_stream->s at -1, because it's not yet valid */
   n_stream->package_window = STREAMWINDOW_START;
   n_stream->deliver_window = STREAMWINDOW_START;
-  if(connection_add(n_stream) < 0) { /* no space, forget it */
-    log_fn(LOG_WARN,"connection_add failed. Dropping.");
-    connection_free(n_stream);
-    return 0;
-  }
 
   log_fn(LOG_DEBUG,"finished adding conn");
 
@@ -1202,6 +1197,7 @@ static int connection_exit_begin_conn(cell_t *cell, circuit_t *circ) {
     if(rend_service_set_connection_addr_port(n_stream, circ) < 0) {
       log_fn(LOG_INFO,"Didn't find rendezvous service (port %d)",n_stream->port);
       connection_mark_for_close(n_stream, END_STREAM_REASON_EXITPOLICY);
+      connection_free(n_stream);
       circuit_mark_for_close(circ); /* knock the whole thing down, somebody screwed up */
       return 0;
     }
@@ -1223,6 +1219,7 @@ static int connection_exit_begin_conn(cell_t *cell, circuit_t *circ) {
     case -1: /* resolve failed */
       log_fn(LOG_INFO,"Resolve failed (%s).", n_stream->address);
       connection_mark_for_close(n_stream, END_STREAM_REASON_RESOLVEFAILED);
+      connection_free(n_stream);
       break;
     case 0: /* resolve added to pending list */
       ;
@@ -1244,9 +1241,9 @@ void connection_exit_connect(connection_t *conn) {
   switch(connection_connect(conn, conn->address, conn->addr, conn->port)) {
     case -1:
       connection_mark_for_close(conn, END_STREAM_REASON_CONNECTFAILED);
+      connection_free(conn);
       return;
     case 0:
-      connection_set_poll_socket(conn);
       conn->state = EXIT_CONN_STATE_CONNECTING;
 
       connection_watch_events(conn, POLLOUT | POLLIN | POLLERR);
@@ -1256,7 +1253,6 @@ void connection_exit_connect(connection_t *conn) {
     /* case 1: fall through */
   }
 
-  connection_set_poll_socket(conn);
   conn->state = EXIT_CONN_STATE_OPEN;
   if(connection_wants_to_flush(conn)) { /* in case there are any queued data cells */
     log_fn(LOG_WARN,"tell roger: newly connected conn had data waiting!");

src/or/connection_or.c

@@ -111,17 +111,11 @@ connection_t *connection_or_connect(routerinfo_t *router) {
   connection_or_init_conn_from_router(conn, router);
   conn->state = OR_CONN_STATE_CONNECTING;
 
-  if(connection_add(conn) < 0) { /* no space, forget it */
-    connection_free(conn);
-    return NULL;
-  }
-
   switch(connection_connect(conn, router->address, router->addr, router->or_port)) {
     case -1:
-      connection_mark_for_close(conn, 0);
+      connection_free(conn);
       return NULL;
     case 0:
-      connection_set_poll_socket(conn);
       connection_watch_events(conn, POLLIN | POLLOUT | POLLERR);
       /* writable indicates finish, readable indicates broken link,
          error indicates broken link on windows */
@@ -129,8 +123,6 @@ connection_t *connection_or_connect(routerinfo_t *router) {
     /* case 1: fall through */
   }
 
-  connection_set_poll_socket(conn);
-
   if(connection_tls_start_handshake(conn, 0) >= 0)
     return conn;
 

src/or/directory.c

@@ -50,11 +50,6 @@ void directory_initiate_command(routerinfo_t *router, int purpose,
 
   conn->purpose = purpose;
 
-  if(connection_add(conn) < 0) { /* no space, forget it */
-    connection_free(conn);
-    return;
-  }
-
   /* queue the command on the outbuf */
   directory_send_command(conn, purpose, payload, payload_len);
 
@@ -67,13 +62,12 @@ void directory_initiate_command(routerinfo_t *router, int purpose,
     switch(connection_connect(conn, conn->address, conn->addr, conn->port)) {
       case -1:
         router_mark_as_down(conn->nickname); /* don't try him again */
-        connection_mark_for_close(conn, 0);
+        connection_free(conn);
         return;
       case 1:
         conn->state = DIR_CONN_STATE_CLIENT_SENDING; /* start flushing conn */
         /* fall through */
       case 0:
-        connection_set_poll_socket(conn);
         connection_watch_events(conn, POLLIN | POLLOUT | POLLERR);
         /* writable indicates finish, readable indicates broken link,
            error indicates broken link in windowsland. */
@@ -91,7 +85,7 @@ void directory_initiate_command(routerinfo_t *router, int purpose,
     }
 
     conn->state = DIR_CONN_STATE_CLIENT_SENDING;
-    connection_set_poll_socket(conn);
+    connection_add(conn);
     connection_start_reading(conn);
   }
 }

src/or/main.c

@@ -54,6 +54,7 @@ int has_completed_circuit=0;
 
 int connection_add(connection_t *conn) {
   tor_assert(conn);
+  tor_assert(conn->s >= 0);
 
   if(nfds >= options.MaxConn-1) {
     log_fn(LOG_WARN,"failing because nfds is too high.");
@@ -61,10 +62,10 @@ int connection_add(connection_t *conn) {
   }
 
   conn->poll_index = nfds;
-  connection_set_poll_socket(conn);
   connection_array[nfds] = conn;
 
   /* zero these out here, because otherwise we'll inherit values from the previously freed one */
+  poll_array[nfds].fd = conn->s;
   poll_array[nfds].events = 0;
   poll_array[nfds].revents = 0;
 
@@ -76,10 +77,6 @@ int connection_add(connection_t *conn) {
   return 0;
 }
 
-void connection_set_poll_socket(connection_t *conn) {
-  poll_array[conn->poll_index].fd = conn->s;
-}
-
 /* Remove the connection from the global list, and remove the
  * corresponding poll entry.  Calling this function will shift the last
  * connection (if any) into the position occupied by conn.

src/or/or.h

@@ -913,7 +913,6 @@ int dns_resolve(connection_t *exitconn);
 
 int connection_add(connection_t *conn);
 int connection_remove(connection_t *conn);
-void connection_set_poll_socket(connection_t *conn);
 
 void get_connection_array(connection_t ***array, int *n);