[tor-commits] [tor/master] Add a backpointer from entry_guard_t to guard_selection_t

nickm at torproject.org nickm at torproject.org
Fri Dec 16 16:26:18 UTC 2016


commit 6dcbc24a4e9da3d46dc9fa1c225982f7088a6e34
Author: Nick Mathewson <nickm at torproject.org>
Date:   Mon Nov 28 10:50:36 2016 -0500

    Add a backpointer from entry_guard_t to guard_selection_t
    
    This is safe, because no entry_guard_t ever outlives its
    guard_selection_t.
    
    I want this because now that multiple guard selections can be active
    during one tor session, we should make sure that any information we
    register about guards is with respect to the selection that they came
    from.
---
 src/or/entrynodes.c | 52 +++++++++++++++++++++++++++++++++++++++++-----------
 src/or/entrynodes.h |  1 +
 2 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c
index 59205a8..e0626cf 100644
--- a/src/or/entrynodes.c
+++ b/src/or/entrynodes.c
@@ -722,6 +722,7 @@ entry_guard_add_to_sample(guard_selection_t *gs,
   guard->is_reachable = GUARD_REACHABLE_MAYBE;
 
   smartlist_add(gs->sampled_entry_guards, guard);
+  guard->in_selection = gs;
   entry_guard_set_filtered_flags(get_options(), gs, guard);
   entry_guards_changed_for_guard_selection(gs);
   return guard;
@@ -1743,6 +1744,8 @@ entry_guard_succeeded(guard_selection_t *gs,
   if (! guard)
     return -1;
 
+  tor_assert(gs == guard->in_selection); // XXXX prop271 remove argument
+
   unsigned newstate =
     entry_guards_note_guard_success(gs, guard, (*guard_state_p)->state);
 
@@ -1772,6 +1775,8 @@ entry_guard_cancel(guard_selection_t *gs,
   if (! guard)
     return;
 
+  tor_assert(gs == guard->in_selection); // XXXX prop271 remove argument
+
   /* XXXX prop271 -- last_tried_to_connect_at will be erroneous here, but this
    * function will only get called in "bug" cases anyway. */
   guard->is_pending = 0;
@@ -1798,6 +1803,8 @@ entry_guard_failed(guard_selection_t *gs,
   if (! guard)
     return;
 
+  tor_assert(gs == guard->in_selection); // XXXX prop271 remove argument
+
   entry_guards_note_guard_failure(gs, guard);
 
   (*guard_state_p)->state = GUARD_CIRC_STATE_DEAD;
@@ -1876,7 +1883,7 @@ circ_state_has_higher_priority(origin_circuit_t *a,
 }
 
 /**
- * Look at all of the origin_circuit_t * objects in <b>all_circuits</b>,
+ * Look at all of the origin_circuit_t * objects in <b>all_circuits_in</b>,
  * and see if any of them that were previously not ready to use for
  * guard-related reasons are now ready to use. Place those circuits
  * in <b>newly_complete_out</b>, and mark them COMPLETE.
@@ -1885,11 +1892,11 @@ circ_state_has_higher_priority(origin_circuit_t *a,
  */
 int
 entry_guards_upgrade_waiting_circuits(guard_selection_t *gs,
-                                      const smartlist_t *all_circuits,
+                                      const smartlist_t *all_circuits_in,
                                       smartlist_t *newly_complete_out)
 {
   tor_assert(gs);
-  tor_assert(all_circuits);
+  tor_assert(all_circuits_in);
   tor_assert(newly_complete_out);
 
   if (! entry_guards_all_primary_guards_are_down(gs)) {
@@ -1904,10 +1911,24 @@ entry_guards_upgrade_waiting_circuits(guard_selection_t *gs,
   int n_complete = 0;
   origin_circuit_t *best_waiting_circuit = NULL;
   origin_circuit_t *best_complete_circuit = NULL;
-  SMARTLIST_FOREACH_BEGIN(all_circuits, origin_circuit_t *, circ) {
+  smartlist_t *all_circuits = smartlist_new();
+  SMARTLIST_FOREACH_BEGIN(all_circuits_in, origin_circuit_t *, circ) {
+    // We filter out circuits that aren't ours, or which we can't
+    // reason about.
     circuit_guard_state_t *state = origin_circuit_get_guard_state(circ);
     if (state == NULL)
       continue;
+    entry_guard_t *guard = entry_guard_handle_get(state->guard);
+    if (!guard || guard->in_selection != gs)
+      continue;
+
+    smartlist_add(all_circuits, circ);
+  } SMARTLIST_FOREACH_END(circ);
+
+  SMARTLIST_FOREACH_BEGIN(all_circuits, origin_circuit_t *, circ) {
+    circuit_guard_state_t *state = origin_circuit_get_guard_state(circ);
+    if BUG((state == NULL))
+      continue;
 
     if (state->state == GUARD_CIRC_STATE_WAITING_FOR_BETTER_GUARD) {
       ++n_waiting;
@@ -1927,7 +1948,7 @@ entry_guards_upgrade_waiting_circuits(guard_selection_t *gs,
   if (! best_waiting_circuit) {
     log_debug(LD_GUARD, "Considered upgrading guard-stalled circuits, "
               "but didn't find any.");
-    return 0;
+    goto no_change;
   }
 
   if (best_complete_circuit) {
@@ -1940,8 +1961,7 @@ entry_guards_upgrade_waiting_circuits(guard_selection_t *gs,
                 "%d complete and %d guard-stalled. At least one complete "
                 "circuit had higher priority, so not upgrading.",
                 n_complete, n_waiting);
-
-      return 0;
+      goto no_change;
     }
   }
 
@@ -1959,7 +1979,7 @@ entry_guards_upgrade_waiting_circuits(guard_selection_t *gs,
     approx_time() - get_nonprimary_guard_connect_timeout();
   SMARTLIST_FOREACH_BEGIN(all_circuits, origin_circuit_t *, circ) {
     circuit_guard_state_t *state = origin_circuit_get_guard_state(circ);
-    if (state == NULL)
+    if (BUG(state == NULL))
       continue;
     if (state->state != GUARD_CIRC_STATE_USABLE_IF_NO_BETTER_GUARD)
       continue;
@@ -1973,7 +1993,7 @@ entry_guards_upgrade_waiting_circuits(guard_selection_t *gs,
               "%d guard-stalled, but %d pending circuit(s) had higher "
               "guard priority, so not upgrading.",
               n_waiting, n_blockers_found);
-    return 0;
+    goto no_change;
   }
 
   /* Okay. We have a best waiting circuit, and we aren't waiting for
@@ -1982,7 +2002,7 @@ entry_guards_upgrade_waiting_circuits(guard_selection_t *gs,
   int n_succeeded = 0;
   SMARTLIST_FOREACH_BEGIN(all_circuits, origin_circuit_t *, circ) {
     circuit_guard_state_t *state = origin_circuit_get_guard_state(circ);
-    if (state == NULL)
+    if (BUG(state == NULL))
       continue;
     if (state->state != GUARD_CIRC_STATE_WAITING_FOR_BETTER_GUARD)
       continue;
@@ -2001,7 +2021,12 @@ entry_guards_upgrade_waiting_circuits(guard_selection_t *gs,
            n_waiting, n_complete, n_succeeded);
 
   tor_assert_nonfatal(n_succeeded >= 1);
+  smartlist_free(all_circuits);
   return 1;
+
+ no_change:
+  smartlist_free(all_circuits);
+  return 0;
 }
 
 /**
@@ -2431,6 +2456,7 @@ entry_guards_load_guards_from_state(or_state_t *state, int set)
                                        GS_TYPE_INFER, 1);
       tor_assert(gs);
       smartlist_add(gs->sampled_entry_guards, guard);
+      guard->in_selection = gs;
     } else {
       entry_guard_free(guard);
     }
@@ -2925,6 +2951,7 @@ add_an_entry_guard(guard_selection_t *gs,
     smartlist_insert(gs->chosen_entry_guards, 0, entry);
   else
     smartlist_add(gs->chosen_entry_guards, entry);
+  entry->in_selection = gs;
 
   control_event_guard(entry->nickname, entry->identity, "NEW");
   control_event_guard_deferred();
@@ -3127,6 +3154,7 @@ remove_all_entry_guards_for_guard_selection(guard_selection_t *gs)
 void
 remove_all_entry_guards(void)
 {
+  // XXXX prop271 this function shouldn't exist, in the new order.
   remove_all_entry_guards_for_guard_selection(get_guard_selection_info());
 }
 
@@ -4037,6 +4065,8 @@ entry_guards_parse_state_for_guard_selection(
       smartlist_free(gs->chosen_entry_guards);
     }
     gs->chosen_entry_guards = new_entry_guards;
+    SMARTLIST_FOREACH(new_entry_guards, entry_guard_t *, e,
+                      e->in_selection = gs);
 
     /* XXX hand new_entry_guards to this func, and move it up a
      * few lines, so we don't have to re-dirty it */
@@ -4429,7 +4459,7 @@ guards_update_all(void)
   if (curr_guard_context->type == GS_TYPE_LEGACY) {
     entry_guards_compute_status(get_options(), approx_time());
   } else {
-    if (entry_guards_update_all(get_guard_selection_info()))
+    if (entry_guards_update_all(curr_guard_context))
       mark_circuits = 1;
   }
 
diff --git a/src/or/entrynodes.h b/src/or/entrynodes.h
index 0164667..97cc4d2 100644
--- a/src/or/entrynodes.h
+++ b/src/or/entrynodes.h
@@ -125,6 +125,7 @@ struct entry_guard_t {
    * Which selection does this guard belong to?
    */
   char *selection_name;
+  guard_selection_t *in_selection;
 
   /* ==== Non-persistent fields. */
   /* == These are used by sampled guards */





More information about the tor-commits mailing list