fix vicious bug in connection_ap_attach_pending that caused it to

never work.
fix vicious bug in choose_good_exit_server that caused it to *skip over*
  pending circuits, and look only at *non-pending circuits*, when choosing
  a good exit node for the new circuit.
bugfix: remove incorrect asserts in circuit_get_newest()


svn:r876

src/or/circuit.c

@@ -211,12 +211,10 @@ circuit_t *circuit_get_newest(connection_t *conn, int must_be_open) {
       }
     }
     if(!newest || newest->timestamp_created < circ->timestamp_created) {
-      assert(circ->n_circ_id);
       newest = circ;
     }
     if(conn && circ->timestamp_dirty &&
        (!leastdirty || leastdirty->timestamp_dirty < circ->timestamp_dirty)) {
-      assert(circ->n_circ_id);
       leastdirty = circ;
     }
   }

src/or/connection.c

@@ -44,7 +44,7 @@ char *conn_state_to_string[][_CONN_TYPE_MAX+1] = {
     "", /* 2 */
     "", /* 3 */
     "awaiting dest info",         /* app, 4 */
-    "waiting for OR connection",       /* 5 */
+    "waiting for safe circuit",        /* 5 */
     "open" },                          /* 6 */
   { "ready" }, /* dir listener, 0 */
   { "",                           /* dir, 0 */

src/or/connection_edge.c

@@ -509,7 +509,7 @@ void connection_ap_attach_pending(void)
   for (i = 0; i < n; ++i) {
     conn = carray[i];
     if (conn->type != CONN_TYPE_AP ||
-        conn->type != AP_CONN_STATE_CIRCUIT_WAIT)
+        conn->state != AP_CONN_STATE_CIRCUIT_WAIT)
       continue;
     switch(connection_ap_handshake_attach_circuit(conn)) {
       case -1: /* it will never work */
@@ -825,6 +825,9 @@ int connection_ap_can_use_exit(connection_t *conn, routerinfo_t *exit)
   assert(conn->type == CONN_TYPE_AP);
   assert(conn->socks_request);
 
+  log_fn(LOG_DEBUG,"considering nickname %s, for address %s / port %d:",
+         exit->nickname, conn->socks_request->address,
+         conn->socks_request->port);
   addr = client_dns_lookup_entry(conn->socks_request->address);
   return router_supports_exit_address(addr, conn->socks_request->port, exit);
 }

src/or/onion.c

@@ -242,7 +242,9 @@ static routerinfo_t *choose_good_exit_server(directory_t *dir)
    * We use this for log messages now, but in the future we may depend on it.
    */
   for (i = 0; i < n_connections; ++i) {
-    if (carray[i]->type == CONN_TYPE_AP && carray[i]->state == AP_CONN_STATE_CIRCUIT_WAIT)
+    if (carray[i]->type == CONN_TYPE_AP &&
+        carray[i]->state == AP_CONN_STATE_CIRCUIT_WAIT &&
+        !carray[i]->marked_for_close)
       ++n_pending_connections;
   }
   log_fn(LOG_DEBUG, "Choosing exit node; %d connections are pending",
@@ -259,34 +261,46 @@ static routerinfo_t *choose_good_exit_server(directory_t *dir)
   for (i = 0; i < dir->n_routers; ++i) { /* iterate over routers */
     if(!dir->routers[i]->is_running) {
       n_supported[i] = n_maybe_supported[i] = -1;
+      log_fn(LOG_DEBUG,"Skipping node %s (index %d) -- directory says it's not running.",
+             dir->routers[i]->nickname, i);
       continue; /* skip routers that are known to be down */
     }
     if(router_exit_policy_rejects_all(dir->routers[i])) {
       n_supported[i] = n_maybe_supported[i] = -1;
+      log_fn(LOG_DEBUG,"Skipping node %s (index %d) -- it rejects all.",
+             dir->routers[i]->nickname, i);
       continue; /* skip routers that reject all */
     }
     n_supported[i] = n_maybe_supported[i] = 0;
     ++n_running_routers;
     for (j = 0; j < n_connections; ++j) { /* iterate over connections */
       if (carray[j]->type != CONN_TYPE_AP ||
-          carray[j]->state == AP_CONN_STATE_CIRCUIT_WAIT ||
+          carray[j]->state != AP_CONN_STATE_CIRCUIT_WAIT ||
           carray[j]->marked_for_close)
         continue; /* Skip everything but APs in CIRCUIT_WAIT */
       switch (connection_ap_can_use_exit(carray[j], dir->routers[i])) 
         {
         case -1:
+          log_fn(LOG_DEBUG,"%s (index %d) would reject this stream.",
+                 dir->routers[i]->nickname, i);
           break; /* would be rejected; try next connection */
         case 0:
           ++n_supported[i];
+          log_fn(LOG_DEBUG,"%s is supported. n_supported[%d] now %d.",
+                 dir->routers[i]->nickname, i, n_supported[i]);
           ; /* Fall through: If it is supported, it is also maybe supported. */
         case 1:
           ++n_maybe_supported[i];
+          log_fn(LOG_DEBUG,"%s is maybe supported. n_maybe_supported[%d] now %d.",
+                 dir->routers[i]->nickname, i, n_maybe_supported[i]);
         }
     } /* End looping over connections. */
     if (n_supported[i] > best_support) { 
       /* If this router is better than previous ones, remember its index
        * and goodness, and start counting how many routers are this good. */
       best_support = n_supported[i]; best_support_idx = i; n_best_support=1;
+      log_fn(LOG_DEBUG,"%s is new best supported option so far.",
+             dir->routers[i]->nickname);
     } else if (n_supported[i] == best_support) {
       /* If this router is _as good_ as the best one, just increment the
        * count of equally good routers.*/
@@ -296,6 +310,8 @@ static routerinfo_t *choose_good_exit_server(directory_t *dir)
     if (n_maybe_supported[i] > best_maybe_support) {
       best_maybe_support = n_maybe_supported[i]; best_maybe_support_idx = i;
       n_best_maybe_support = 1;
+      log_fn(LOG_DEBUG,"%s is new best maybe-supported option so far.",
+             dir->routers[i]->nickname);
     } else if (n_maybe_supported[i] == best_maybe_support) {
       ++n_best_maybe_support;
     }