when we were cannibalizing a circuit with a particular exit

node in mind, we weren't checking to see if that exit node
was already present earlier in the circuit. oops.


svn:r5357

src/or/circuitlist.c

@@ -99,7 +99,7 @@ circuit_set_circid_orconn(circuit_t *circ, uint16_t id,
     _last_circid_orconn_ent = NULL;
   }
 
-  if (old_conn) {
+  if (old_conn) { /* we may need to remove it from the conn-circid map */
     search.circ_id = old_id;
     search.or_conn = old_conn;
     found = RB_FIND(orconn_circid_tree, &orconn_circid_circuit_map, &search);
@@ -112,6 +112,7 @@ circuit_set_circid_orconn(circuit_t *circ, uint16_t id,
   if (conn == NULL)
     return;
 
+  /* now add the new one to the conn-circid map */
   search.circ_id = id;
   search.or_conn = conn;
   found = RB_FIND(orconn_circid_tree, &orconn_circid_circuit_map, &search);
@@ -549,16 +550,19 @@ circuit_get_rendezvous(const char *cookie)
 }
 
 /** Return a circuit that is open, has specified <b>purpose</b>,
- * has a timestamp_dirty value of 0, and is uptime/capacity/internal
- * if required; or NULL if no circuit fits this description.
+ * has a timestamp_dirty value of 0, is uptime/capacity/internal
+ * if required, and if info is defined, does not already use info
+ * as any of its hops; or NULL if no circuit fits this description.
  *
  * Avoid returning need_uptime circuits if not necessary.
+ *
  * FFFF As a more important goal, not yet implemented, avoid returning
  * internal circuits if not necessary.
  */
 circuit_t *
-circuit_get_clean_open(uint8_t purpose, int need_uptime,
-                       int need_capacity, int internal)
+circuit_find_to_cannibalize(uint8_t purpose, extend_info_t *info,
+                            int need_uptime,
+                            int need_capacity, int internal)
 {
   circuit_t *circ;
   circuit_t *best=NULL;
@@ -574,8 +578,19 @@ circuit_get_clean_open(uint8_t purpose, int need_uptime,
         (!need_uptime || circ->build_state->need_uptime) &&
         (!need_capacity || circ->build_state->need_capacity) &&
         (!internal || circ->build_state->is_internal)) {
+      if (info) {
+        /* need to make sure we don't duplicate hops */
+        crypt_path_t *hop = circ->cpath;
+        do {
+          if (!memcmp(hop->extend_info->identity_digest,
+                      info->identity_digest, DIGEST_LEN))
+            goto next;
+          hop=hop->next;
+        } while (hop!=circ->cpath);
+      }
       if (!best || (best->build_state->need_uptime && !need_uptime))
         best = circ;
+      next:
     }
   }
   return best;

src/or/circuituse.c

@@ -756,8 +756,9 @@ circuit_launch_by_extend_info(uint8_t purpose, extend_info_t *info,
   if ((info || purpose != CIRCUIT_PURPOSE_C_GENERAL) &&
       purpose != CIRCUIT_PURPOSE_TESTING) {
     /* see if there are appropriate circs available to cannibalize. */
-    if ((circ = circuit_get_clean_open(CIRCUIT_PURPOSE_C_GENERAL, need_uptime,
-                                       need_capacity, internal))) {
+    circ = circuit_find_to_cannibalize(CIRCUIT_PURPOSE_C_GENERAL, info,
+                                       need_uptime, need_capacity, internal);
+    if (circ) {
       info(LD_CIRC,"Cannibalizing circ '%s' for purpose %d",
              build_state_get_exit_nickname(circ->build_state), purpose);
       circ->purpose = purpose;
@@ -779,7 +780,7 @@ circuit_launch_by_extend_info(uint8_t purpose, extend_info_t *info,
             return NULL;
           break;
         default:
-          warn(LD_BUG, "Bug: unexpected purpose %d when cannibalizing a general circ.", purpose);
+          warn(LD_BUG, "Bug: unexpected purpose %d when cannibalizing a circ.", purpose);
           tor_fragile_assert();
           return NULL;
       }

src/or/or.h

@@ -1443,8 +1443,9 @@ circuit_t *circuit_get_by_rend_query_and_purpose(const char *rend_query, uint8_t
 circuit_t *circuit_get_next_by_pk_and_purpose(circuit_t *start,
                                          const char *digest, uint8_t purpose);
 circuit_t *circuit_get_rendezvous(const char *cookie);
-circuit_t *circuit_get_clean_open(uint8_t purpose, int need_uptime,
-                                  int need_capacity, int internal);
+circuit_t *circuit_find_to_cannibalize(uint8_t purpose, extend_info_t *info,
+                                       int need_uptime,
+                                       int need_capacity, int internal);
 void circuit_mark_all_unused_circs(void);
 void circuit_expire_all_dirty_circs(void);
 void _circuit_mark_for_close(circuit_t *circ, int line, const char *file);