Make "connected" a different case from "finished_flushing"; always close_immediate whhen connect() fails.

svn:r1852

src/or/connection.c

@@ -77,6 +77,8 @@ char *conn_state_to_string[][_CONN_TYPE_MAX+1] = {
 static int connection_init_accepted_conn(connection_t *conn);
 static int connection_handle_listener_read(connection_t *conn, int new_type);
 static int connection_receiver_bucket_should_increase(connection_t *conn);
+static int connection_finished_flushing(connection_t *conn);
+static int connection_finished_connecting(connection_t *conn);
 
 /**************************************************************/
 
@@ -775,13 +777,34 @@ int connection_outbuf_too_full(connection_t *conn) {
  * return 0.
  */
 int connection_handle_write(connection_t *conn) {
+  int e, len=sizeof(e);
 
   tor_assert(!connection_is_listener(conn));
 
   conn->timestamp_lastwritten = time(NULL);
 
-  if (connection_speaks_cells(conn) &&
+  /* Sometimes, "writeable" means "connected". */
-      conn->state != OR_CONN_STATE_CONNECTING) {
+  if (connection_state_is_connecting(conn)) {
+    if (getsockopt(conn->s, SOL_SOCKET, SO_ERROR, (void*)&e, &len) < 0) {
+      /* not yet */
+      if(!ERRNO_IS_CONN_EINPROGRESS(tor_socket_errno(conn->s))) {
+        log_fn(LOG_DEBUG,"in-progress connect failed. Removing.");
+        /* The reason here only applies to exit connections, but it's
+         * harmless to set it elsewhere. */
+        connection_close_immediate(conn);
+        connection_mark_for_close(conn,END_STREAM_REASON_CONNECTFAILED);
+        if (conn->nickname)
+          router_mark_as_down(conn->nickname);
+        return -1;
+      } else {
+        return 0; /* no change, see if next time is better */
+      }
+    }
+    /* The connection is successful. */
+    return connection_finished_connecting(conn);
+  }
+
+  if (connection_speaks_cells(conn)) {
     if (conn->state == OR_CONN_STATE_HANDSHAKING) {
       connection_stop_writing(conn);
       if(connection_tls_continue_handshake(conn) < 0) {
@@ -827,7 +850,6 @@ int connection_handle_write(connection_t *conn) {
       connection_mark_for_close(conn, END_STREAM_REASON_MISC);
       return -1;
     }
-    /* conns in CONNECTING state will fall through... */
   }
 
   if(!connection_wants_to_flush(conn)) /* it's done flushing */
@@ -1031,6 +1053,24 @@ int connection_state_is_open(connection_t *conn) {
   return 0;
 }
 
+int connection_state_is_connecting(connection_t *conn) {
+  tor_assert(conn);
+
+  if (conn->marked_for_close)
+    return 0;
+  switch (conn->type)
+    {
+    case CONN_TYPE_OR:
+      return conn->state == OR_CONN_STATE_CONNECTING;
+    case CONN_TYPE_EXIT:
+      return conn->state == EXIT_CONN_STATE_CONNECTING;
+    case CONN_TYPE_DIR:
+      return conn->state == DIR_CONN_STATE_CONNECTING;
+    }
+
+  return 0;
+}
+
 /** Write a destroy cell with circ ID <b>circ_id</b> onto OR connection
  * <b>conn</b>.
  *
@@ -1083,7 +1123,7 @@ int connection_process_inbuf(connection_t *conn) {
  * This function just passes conn to the connection-specific
  * connection_*_finished_flushing() function.
  */
-int connection_finished_flushing(connection_t *conn) {
+static int connection_finished_flushing(connection_t *conn) {
 
   tor_assert(conn);
 
@@ -1107,6 +1147,29 @@ int connection_finished_flushing(connection_t *conn) {
   }
 }
 
+/** Called when our attempt to connect() to another server has just
+ * succeeded.
+ *
+ * This function just passes conn to the connection-specific
+ * connection_*_finished_connecting() function.
+ */
+static int connection_finished_connecting(connection_t *conn)
+{
+  tor_assert(conn);
+  switch (conn->type)
+    {
+    case CONN_TYPE_OR:
+      return connection_or_finished_connecting(conn);
+    case CONN_TYPE_EXIT:
+      return connection_edge_finished_connecting(conn);
+    case CONN_TYPE_DIR:
+      return connection_dir_finished_connecting(conn);
+    default:
+      tor_assert(0);
+      return -1;
+  }
+}
+
 /** Verify that connection <b>conn</b> has all of its invariants
  * correct. Trigger an assert if anything is invalid.
  */

src/or/connection_edge.c

@@ -534,10 +534,6 @@ int connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ,
 /** Connection <b>conn</b> has finished writing and has no bytes left on
  * its outbuf.
  *
- * If it's in state 'connecting', then take a look at the socket, and
- * take appropriate actions (such as sending back a relay 'connected'
- * cell) if the connect succeeded.
- *
  * If it's in state 'open', stop writing, consider responding with a
  * sendme, and return.
  * Otherwise, stop writing and return.
@@ -546,47 +542,10 @@ int connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ,
  * return 0.
  */
 int connection_edge_finished_flushing(connection_t *conn) {
-  unsigned char connected_payload[4];
-  int e, len=sizeof(e);
-
   tor_assert(conn);
   tor_assert(conn->type == CONN_TYPE_AP || conn->type == CONN_TYPE_EXIT);
 
   switch(conn->state) {
-    case EXIT_CONN_STATE_CONNECTING:
-      if (getsockopt(conn->s, SOL_SOCKET, SO_ERROR, (void*)&e, &len) < 0)  { /* not yet */
-        if(!ERRNO_IS_CONN_EINPROGRESS(tor_socket_errno(conn->s))) {
-          /* yuck. kill it. */
-          log_fn(LOG_DEBUG,"in-progress exit connect failed. Removing.");
-          connection_mark_for_close(conn, END_STREAM_REASON_CONNECTFAILED);
-          return -1;
-        } else {
-          log_fn(LOG_DEBUG,"in-progress exit connect still waiting.");
-          return 0; /* no change, see if next time is better */
-        }
-      }
-      /* the connect has finished. */
-
-      log_fn(LOG_INFO,"Exit connection to %s:%u established.",
-          conn->address,conn->port);
-
-      conn->state = EXIT_CONN_STATE_OPEN;
-      connection_watch_events(conn, POLLIN); /* stop writing, continue reading */
-      if(connection_wants_to_flush(conn)) /* in case there are any queued relay cells */
-        connection_start_writing(conn);
-      /* deliver a 'connected' relay cell back through the circuit. */
-      if(connection_edge_is_rendezvous_stream(conn)) {
-        if(connection_edge_send_command(conn, circuit_get_by_conn(conn),
-           RELAY_COMMAND_CONNECTED, NULL, 0, conn->cpath_layer) < 0)
-          return 0; /* circuit is closed, don't continue */
-      } else {
-        *(uint32_t*)connected_payload = htonl(conn->addr);
-        if(connection_edge_send_command(conn, circuit_get_by_conn(conn),
-           RELAY_COMMAND_CONNECTED, connected_payload, 4, conn->cpath_layer) < 0)
-          return 0; /* circuit is closed, don't continue */
-      }
-      tor_assert(conn->package_window > 0);
-      return connection_edge_process_inbuf(conn); /* in case the server has written anything */
     case AP_CONN_STATE_OPEN:
     case EXIT_CONN_STATE_OPEN:
       connection_stop_writing(conn);
@@ -605,6 +564,40 @@ int connection_edge_finished_flushing(connection_t *conn) {
   return 0;
 }
 
+/** Connected handler for exit connections: start writing pending
+ * data, deliver 'CONNECTED' relay cells as appropriate, and check
+ * any pending data that may have been received. */
+int connection_edge_finished_connecting(connection_t *conn)
+{
+  unsigned char connected_payload[4];
+
+  tor_assert(conn);
+  tor_assert(conn->type == CONN_TYPE_EXIT);
+  tor_assert(conn->state == EXIT_CONN_STATE_CONNECTING);
+
+
+  log_fn(LOG_INFO,"Exit connection to %s:%u established.",
+         conn->address,conn->port);
+
+  conn->state = EXIT_CONN_STATE_OPEN;
+  connection_watch_events(conn, POLLIN); /* stop writing, continue reading */
+  if(connection_wants_to_flush(conn)) /* in case there are any queued relay cells */
+    connection_start_writing(conn);
+  /* deliver a 'connected' relay cell back through the circuit. */
+  if(connection_edge_is_rendezvous_stream(conn)) {
+    if(connection_edge_send_command(conn, circuit_get_by_conn(conn),
+                                    RELAY_COMMAND_CONNECTED, NULL, 0, conn->cpath_layer) < 0)
+      return 0; /* circuit is closed, don't continue */
+  } else {
+    *(uint32_t*)connected_payload = htonl(conn->addr);
+    if(connection_edge_send_command(conn, circuit_get_by_conn(conn),
+         RELAY_COMMAND_CONNECTED, connected_payload, 4, conn->cpath_layer) < 0)
+      return 0; /* circuit is closed, don't continue */
+  }
+  tor_assert(conn->package_window > 0);
+  return connection_edge_process_inbuf(conn); /* in case the server has written anything */
+}
+
 uint64_t stats_n_data_cells_packaged = 0;
 uint64_t stats_n_data_bytes_packaged = 0;
 uint64_t stats_n_data_cells_received = 0;

src/or/connection_or.c

@@ -59,50 +59,41 @@ int connection_or_process_inbuf(connection_t *conn) {
 /** Connection <b>conn</b> has finished writing and has no bytes left on
  * its outbuf.
  *
- * If it's in state "connecting", then take a look at the socket, and
- * begin the tls handshake if the connect succeeded.
- *
  * Otherwise it's in state "open": stop writing and return.
  *
  * If <b>conn</b> is broken, mark it for close and return -1, else
  * return 0.
  */
 int connection_or_finished_flushing(connection_t *conn) {
-  int e, len=sizeof(e);
-
   tor_assert(conn && conn->type == CONN_TYPE_OR);
+
   assert_connection_ok(conn,0);
 
-  switch(conn->state) {
+  if (conn->state != OR_CONN_STATE_OPEN) {
-    case OR_CONN_STATE_CONNECTING:
+    log_fn(LOG_WARN,"BUG: called in unexpected state %d",conn->state);
-      if (getsockopt(conn->s, SOL_SOCKET, SO_ERROR, (void*)&e, &len) < 0) {
+    return -1;
-        /* not yet */
-        if(!ERRNO_IS_CONN_EINPROGRESS(tor_socket_errno(conn->s))) {
-          log_fn(LOG_DEBUG,"in-progress connect failed. Removing.");
-          connection_mark_for_close(conn,0);
-          return -1;
-        } else {
-          return 0; /* no change, see if next time is better */
-        }
-      }
-      /* the connect has finished. */
-
-      log_fn(LOG_INFO,"OR connect() to router %s:%u finished.",
-          conn->address,conn->port);
-
-      if(connection_tls_start_handshake(conn, 0) < 0) {
-        /* TLS handhaking error of some kind. */
-        connection_mark_for_close(conn,0);
-        return -1;
-      }
-      return 0;
-    case OR_CONN_STATE_OPEN:
-      connection_stop_writing(conn);
-      return 0;
-    default:
-      log_fn(LOG_WARN,"BUG: called in unexpected state %d",conn->state);
-      return 0;
   }
+
+  connection_stop_writing(conn);
+  return 0;
+}
+
+/** Connected handler for OR connections: begin the TLS handshake.
+ */
+int connection_or_finished_connecting(connection_t *conn)
+{
+  tor_assert(conn && conn->type == CONN_TYPE_OR);
+  tor_assert(conn->state == OR_CONN_STATE_CONNECTING);
+
+  log_fn(LOG_INFO,"OR connect() to router %s:%u finished.",
+         conn->address,conn->port);
+
+  if(connection_tls_start_handshake(conn, 0) < 0) {
+    /* TLS handhaking error of some kind. */
+    connection_mark_for_close(conn,0);
+    return -1;
+  }
+  return 0;
 }
 
 /** Initialize <b>conn</b> to include all the relevant data from <b>router</b>.

src/or/directory.c

@@ -523,8 +523,8 @@ static int directory_handle_command(connection_t *conn) {
 }
 
 /** Write handler for directory connections; called when all data has
- * been flushed.  Handle a completed connection: close the connection
+ * been flushed.  Close the connection or wait for a response as
- * or wait for a response as appropriate.
+ * appropriate.
  */
 int connection_dir_finished_flushing(connection_t *conn) {
   int e, len=sizeof(e);
@@ -532,24 +532,6 @@ int connection_dir_finished_flushing(connection_t *conn) {
   tor_assert(conn && conn->type == CONN_TYPE_DIR);
 
   switch(conn->state) {
-    case DIR_CONN_STATE_CONNECTING:
-      if (getsockopt(conn->s, SOL_SOCKET, SO_ERROR, (void*)&e, &len) < 0)  { /* not yet */
-        if(!ERRNO_IS_CONN_EINPROGRESS(tor_socket_errno(conn->s))) {
-          log_fn(LOG_DEBUG,"in-progress connect failed. Removing.");
-          router_mark_as_down(conn->nickname); /* don't try him again */
-          connection_mark_for_close(conn,0);
-          return -1;
-        } else {
-          return 0; /* no change, see if next time is better */
-        }
-      }
-      /* the connect has finished. */
-
-      log_fn(LOG_INFO,"Dir connection to router %s:%u established.",
-          conn->address,conn->port);
-
-      conn->state = DIR_CONN_STATE_CLIENT_SENDING; /* start flushing conn */
-      return 0;
     case DIR_CONN_STATE_CLIENT_SENDING:
       log_fn(LOG_DEBUG,"client finished sending command.");
       conn->state = DIR_CONN_STATE_CLIENT_READING;
@@ -566,6 +548,20 @@ int connection_dir_finished_flushing(connection_t *conn) {
   return 0;
 }
 
+/** Connected handler for directory connections: begin sending data to the
+ * server */
+int connection_dir_finished_connecting(connection_t *conn)
+{
+  tor_assert(conn && conn->type == CONN_TYPE_DIR);
+  tor_assert(conn->state == DIR_CONN_STATE_CONNECTING);
+
+  log_fn(LOG_INFO,"Dir connection to router %s:%u established.",
+         conn->address,conn->port);
+
+  conn->state = DIR_CONN_STATE_CLIENT_SENDING; /* start flushing conn */
+  return 0;
+}
+
 /*
   Local Variables:
   mode:c

src/or/or.h

@@ -1011,11 +1011,11 @@ connection_t *connection_get_by_type_rendquery(int type, const char *rendquery);
    tor_tls_get_pending_bytes((conn)->tls))
 int connection_is_listener(connection_t *conn);
 int connection_state_is_open(connection_t *conn);
+int connection_state_is_connecting(connection_t *conn);
 
 int connection_send_destroy(uint16_t circ_id, connection_t *conn);
 
 int connection_process_inbuf(connection_t *conn);
-int connection_finished_flushing(connection_t *conn);
 
 void assert_connection_ok(connection_t *conn, time_t now);
 
@@ -1033,6 +1033,7 @@ int connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ,
                                        connection_t *conn,
                                        crypt_path_t *layer_hint);
 int connection_edge_finished_flushing(connection_t *conn);
+int connection_edge_finished_connecting(connection_t *conn);
 
 int connection_edge_package_raw_inbuf(connection_t *conn);
 
@@ -1062,6 +1063,7 @@ void client_dns_clean(void);
 
 int connection_or_process_inbuf(connection_t *conn);
 int connection_or_finished_flushing(connection_t *conn);
+int connection_or_finished_connecting(connection_t *conn);
 
 connection_t *connection_or_connect(routerinfo_t *router);
 
@@ -1086,6 +1088,7 @@ void directory_initiate_command(routerinfo_t *router, int purpose,
                                 const char *payload, int payload_len);
 int connection_dir_process_inbuf(connection_t *conn);
 int connection_dir_finished_flushing(connection_t *conn);
+int connection_dir_finished_connecting(connection_t *conn);
 
 /********************************* dns.c ***************************/