[tor-commits] [tor/master] avoid following through on a consensus fetch if we have one already arriving

[nickm at torproject.org]

commit 91c58013be735584acc674a60bb18d1efb38f25a
Author: Roger Dingledine <arma at torproject.org>
Date:   Wed Apr 13 01:08:17 2016 -0400

    avoid following through on a consensus fetch if we have one already arriving
---
 src/or/circuituse.c    | 19 ++++++++++++++++++
 src/or/networkstatus.c | 53 ++++++++++++++++++++++++++++++++++----------------
 src/or/networkstatus.h |  1 +
 3 files changed, 56 insertions(+), 17 deletions(-)

diff --git a/src/or/circuituse.c b/src/or/circuituse.c
index 31003ea..f32763a 100644
--- a/src/or/circuituse.c
+++ b/src/or/circuituse.c
@@ -2355,6 +2355,25 @@ connection_ap_handshake_attach_circuit(entry_connection_t *conn)
     /* we're a general conn */
     origin_circuit_t *circ=NULL;
 
+    /* Are we linked to a dir conn that aims to fetch a consensus?
+     * We check here because this conn might no longer be needed. */
+    if (base_conn->linked_conn &&
+        base_conn->linked_conn->type == CONN_TYPE_DIR &&
+        base_conn->linked_conn->purpose == DIR_PURPOSE_FETCH_CONSENSUS) {
+
+      /* Yes we are. Is there a consensus fetch farther along than us? */
+      if (networkstatus_consensus_is_already_downloading(
+            TO_DIR_CONN(base_conn->linked_conn)->requested_resource)) {
+        /* We're doing the "multiple consensus fetch attempts" game from
+         * proposal 210, and we're late to the party. Just close this conn.
+         * The circuit and TLS conn that we made will time out after a while
+         * if nothing else wants to use them. */
+        log_info(LD_DIR, "Closing extra consensus fetch (to %s) since one "
+                 "is already downloading.", base_conn->linked_conn->address);
+        return -1;
+      }
+    }
+
     if (conn->chosen_exit_name) {
       const node_t *node = node_get_by_nickname(conn->chosen_exit_name, 1);
       int opt = conn->chosen_exit_optional;
diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c
index 51c985e..e90603e 100644
--- a/src/or/networkstatus.c
+++ b/src/or/networkstatus.c
@@ -1349,27 +1349,46 @@ networkstatus_consensus_has_excess_connections(void)
   return 0;
 }
 
+/* Is there a consensus fetch for flavor <b>resource</b> that's far
+ * enough along to be attached to a circuit? */
+int
+networkstatus_consensus_is_already_downloading(const char *resource)
+{
+  int answer = 0;
+
+  /* First, get a list of all the dir conns that are fetching a consensus,
+   * fetching *this* consensus, and are in state "reading" (meaning they
+   * have already flushed their request onto the socks connection). */
+  smartlist_t *fetching_conns =
+    connection_dir_list_by_purpose_resource_and_state(
+      DIR_PURPOSE_FETCH_CONSENSUS, resource, DIR_CONN_STATE_CLIENT_READING);
+
+  /* Then, walk through each conn, to see if its linked socks connection
+   * is in an attached state. We have to check this separately, since with
+   * the optimistic data feature, fetches can send their request to the
+   * socks connection and go into state 'reading', even before they're
+   * attached to any circuit. */
+  SMARTLIST_FOREACH_BEGIN(fetching_conns, dir_connection_t *, dirconn) {
+    /* Do any of these other dir conns have a linked socks conn that is
+     * attached to a circuit already? */
+    connection_t *base = TO_CONN(dirconn);
+    if (base->linked_conn &&
+        base->linked_conn->type == CONN_TYPE_AP &&
+        !AP_CONN_STATE_IS_UNATTACHED(base->linked_conn->state))
+      answer = 1;
+  } SMARTLIST_FOREACH_END(dirconn);
+  smartlist_free(fetching_conns);
+
+  return answer;
+}
+
 /* Is tor currently downloading a consensus of the usable flavor? */
 int
 networkstatus_consensus_is_downloading_usable_flavor(void)
 {
-  const char *usable_resource = networkstatus_get_flavor_name(
-                                                  usable_consensus_flavor());
-  const int consens_conn_usable_count =
-              connection_dir_count_by_purpose_and_resource(
-                                               DIR_PURPOSE_FETCH_CONSENSUS,
-                                               usable_resource);
-
-  const int connect_consens_conn_usable_count =
-              connection_dir_count_by_purpose_resource_and_state(
-                                                DIR_PURPOSE_FETCH_CONSENSUS,
-                                                usable_resource,
-                                                DIR_CONN_STATE_CONNECTING);
-  if (connect_consens_conn_usable_count < consens_conn_usable_count) {
-    return 1;
-  }
-
-  return 0;
+  const char *resource =
+    networkstatus_get_flavor_name(usable_consensus_flavor());
+  return networkstatus_consensus_is_already_downloading(resource);
 }
 
 /** Given two router status entries for the same router identity, return 1 if
diff --git a/src/or/networkstatus.h b/src/or/networkstatus.h
index f2f8af5..e1cd10b 100644
--- a/src/or/networkstatus.h
+++ b/src/or/networkstatus.h
@@ -76,6 +76,7 @@ int networkstatus_consensus_can_use_multiple_directories(
 int networkstatus_consensus_can_use_extra_fallbacks(
                                                 const or_options_t *options);
 int networkstatus_consensus_has_excess_connections(void);
+int networkstatus_consensus_is_already_downloading(const char *resource);
 int networkstatus_consensus_is_downloading_usable_flavor(void);
 
 #define NSSET_FROM_CACHE 1