Merge remote-tracking branch 'public/ticket20921'

changes/bug21118

@@ -0,0 +1,6 @@
+  o Major bugfixes (client, guard, crash):
+    - In circuit_get_global_origin_list(), return the actual list of
+      origin circuits. The previous version of this code returned
+      the list of all the circuits, and could have caused strange bugs,
+      including possible crashes.  Fixes bug 21118; bugfix on 0.3.0.1-alpha.
+

changes/ticket20921

@@ -0,0 +1,4 @@
+  o Code simplification and refactoring:
+    - Refactor code to manipulate global_origin_circuit_list into separate
+      functions. Closes ticket 20921.
+

src/or/circuitlist.c

@@ -509,6 +509,39 @@ circuit_count_pending_on_channel(channel_t *chan)
   return cnt;
 }
 
+/** Remove <b>origin_circ</b> from the global list of origin circuits.
+ * Called when we are freeing a circuit.
+ */
+static void
+circuit_remove_from_origin_circuit_list(origin_circuit_t *origin_circ)
+{
+  int origin_idx = origin_circ->global_origin_circuit_list_idx;
+  if (origin_idx < 0)
+    return;
+  origin_circuit_t *c2;
+  tor_assert(origin_idx <= smartlist_len(global_origin_circuit_list));
+  c2 = smartlist_get(global_origin_circuit_list, origin_idx);
+  tor_assert(origin_circ == c2);
+  smartlist_del(global_origin_circuit_list, origin_idx);
+  if (origin_idx < smartlist_len(global_origin_circuit_list)) {
+    origin_circuit_t *replacement =
+      smartlist_get(global_origin_circuit_list, origin_idx);
+    replacement->global_origin_circuit_list_idx = origin_idx;
+  }
+  origin_circ->global_origin_circuit_list_idx = -1;
+}
+
+/** Add <b>origin_circ</b> to the global list of origin circuits. Called
+ * when creating the circuit. */
+static void
+circuit_add_to_origin_circuit_list(origin_circuit_t *origin_circ)
+{
+  tor_assert(origin_circ->global_origin_circuit_list_idx == -1);
+  smartlist_t *lst = circuit_get_global_origin_circuit_list();
+  smartlist_add(lst, origin_circ);
+  origin_circ->global_origin_circuit_list_idx = smartlist_len(lst) - 1;
+}
+
 /** Detach from the global circuit list, and deallocate, all
  * circuits that have been marked for close.
  */
@@ -533,15 +566,7 @@ circuit_close_all_marked(void)
 
     /* Remove it from the origin circuit list, if appropriate. */
     if (CIRCUIT_IS_ORIGIN(circ)) {
-      origin_circuit_t *origin_circ = TO_ORIGIN_CIRCUIT(circ);
-      int origin_idx = origin_circ->global_origin_circuit_list_idx;
-      smartlist_del(global_origin_circuit_list, origin_idx);
-      if (origin_idx < smartlist_len(global_origin_circuit_list)) {
-        origin_circuit_t *replacement =
-          smartlist_get(global_origin_circuit_list, origin_idx);
-        replacement->global_origin_circuit_list_idx = origin_idx;
-      }
-      origin_circ->global_origin_circuit_list_idx = -1;
+      circuit_remove_from_origin_circuit_list(TO_ORIGIN_CIRCUIT(circ));
     }
 
     circuit_about_to_free(circ);
@@ -566,7 +591,7 @@ circuit_get_global_origin_circuit_list(void)
 {
   if (NULL == global_origin_circuit_list)
     global_origin_circuit_list = smartlist_new();
-  return global_circuitlist;
+  return global_origin_circuit_list;
 }
 
 /** Function to make circ-\>state human-readable */
@@ -811,11 +836,8 @@ origin_circuit_new(void)
   init_circuit_base(TO_CIRCUIT(circ));
 
   /* Add to origin-list. */
-  if (!global_origin_circuit_list)
-    global_origin_circuit_list = smartlist_new();
-  smartlist_add(global_origin_circuit_list, circ);
-  circ->global_origin_circuit_list_idx =
-    smartlist_len(global_origin_circuit_list) - 1;
+  circ->global_origin_circuit_list_idx = -1;
+  circuit_add_to_origin_circuit_list(circ);
 
   circuit_build_times_update_last_circ(get_circuit_build_times_mutable());
 
@@ -875,16 +897,7 @@ circuit_free(circuit_t *circ)
     memlen = sizeof(origin_circuit_t);
     tor_assert(circ->magic == ORIGIN_CIRCUIT_MAGIC);
 
-    if (ocirc->global_origin_circuit_list_idx != -1) {
-      int idx = ocirc->global_origin_circuit_list_idx;
-      origin_circuit_t *c2 = smartlist_get(global_origin_circuit_list, idx);
-      tor_assert(c2 == ocirc);
-      smartlist_del(global_origin_circuit_list, idx);
-      if (idx < smartlist_len(global_origin_circuit_list)) {
-        c2 = smartlist_get(global_origin_circuit_list, idx);
-        c2->global_origin_circuit_list_idx = idx;
-      }
-    }
+    circuit_remove_from_origin_circuit_list(ocirc);
 
     if (ocirc->build_state) {
         extend_info_free(ocirc->build_state->chosen_exit);