Stop checking cached bridge descriptors for usable bridges

Stop checking for bridge descriptors when we actually want to know if
any bridges are usable. This avoids potential bootstrapping issues.
Fixes bug 24367; bugfix on 0.2.0.3-alpha.

Stop stalling when bridges are changed at runtime. Stop stalling when
old bridge descriptors are cached, but they are not in use.
Fixes bug 24367; bugfix on 23347 in 0.3.2.1-alpha.

changes/bug24367

@@ -0,0 +1,7 @@
+  o Minor bugfixes (bridge clients, bootstrap):
+    - Stop checking for bridge descriptors when we actually want to know if
+      any bridges are usable. This avoids potential bootstrapping issues.
+      Fixes bug 24367; bugfix on 0.2.0.3-alpha.
+    - Stop stalling when bridges are changed at runtime. Stop stalling when
+      old bridge descriptors are cached, but they are not in use.
+      Fixes bug 24367; bugfix on 23347 in 0.3.2.1-alpha.

src/or/bridges.c

@@ -835,34 +835,6 @@ learned_bridge_descriptor(routerinfo_t *ri, int from_cache)
   }
 }
 
-/** Return the number of bridges that have descriptors that
- * are marked with purpose 'bridge' and are running.
- *
- * We use this function to decide if we're ready to start building
- * circuits through our bridges, or if we need to wait until the
- * directory "server/authority" requests finish. */
-MOCK_IMPL(int,
-any_bridge_descriptors_known, (void))
-{
-  if (BUG(!get_options()->UseBridges)) {
-    return 0;
-  }
-
-  if (!bridge_list)
-    return 0;
-
-  SMARTLIST_FOREACH_BEGIN(bridge_list, bridge_info_t *, bridge) {
-    const node_t *node;
-    if (!tor_digest_is_zero(bridge->identity) &&
-        (node = node_get_by_id(bridge->identity)) != NULL &&
-        node->ri) {
-      return 1;
-    }
-  } SMARTLIST_FOREACH_END(bridge);
-
-  return 0;
-}
-
 /** Return a smartlist containing all bridge identity digests */
 MOCK_IMPL(smartlist_t *,
 list_bridge_identities, (void))

src/or/bridges.h

@@ -45,7 +45,6 @@ void bridge_add_from_config(struct bridge_line_t *bridge_line);
 void retry_bridge_descriptor_fetch_directly(const char *digest);
 void fetch_bridge_descriptors(const or_options_t *options, time_t now);
 void learned_bridge_descriptor(routerinfo_t *ri, int from_cache);
-MOCK_DECL(int, any_bridge_descriptors_known, (void));
 const smartlist_t *get_socks_args_by_bridge_addrport(const tor_addr_t *addr,
                                                      uint16_t port);
 

src/or/circuituse.c

@@ -2084,7 +2084,7 @@ circuit_get_open_circ_or_launch(entry_connection_t *conn,
                "used client functionality lately" :
                "received a consensus with exits",
                options->UseBridges ? "bridges" : "entrynodes");
-      } else if (!options->UseBridges || any_bridge_descriptors_known()) {
+      } else if (!options->UseBridges || num_bridges_usable() > 0) {
         log_fn(severity, LD_APP|LD_DIR,
                "Application request when we haven't %s. "
                "Optimistically trying directory fetches again.",

src/or/directory.c

@@ -5338,7 +5338,7 @@ find_dl_schedule(const download_status_t *dls, const or_options_t *options)
       }
     case DL_SCHED_BRIDGE:
       /* A bridge client downloading bridge descriptors */
-      if (options->UseBridges && any_bridge_descriptors_known()) {
+      if (options->UseBridges && num_bridges_usable() > 0) {
         /* A bridge client with one or more running bridges */
         return options->TestingBridgeDownloadSchedule;
       } else {

src/or/entrynodes.c

@@ -3136,9 +3136,12 @@ entry_list_is_constrained(const or_options_t *options)
 
 /** Return the number of bridges that have descriptors that are marked with
  * purpose 'bridge' and are running.
- */
+ *
-int
+ * We use this function to decide if we're ready to start building
-num_bridges_usable(void)
+ * circuits through our bridges, or if we need to wait until the
+ * directory "server/authority" requests finish. */
+MOCK_IMPL(int,
+num_bridges_usable,(void))
 {
   int n_options = 0;
 

src/or/entrynodes.h

@@ -383,8 +383,7 @@ void entry_guards_note_internet_connectivity(guard_selection_t *gs);
 
 int update_guard_selection_choice(const or_options_t *options);
 
-/* Used by bridges.c only. */
+MOCK_DECL(int,num_bridges_usable,(void));
-int num_bridges_usable(void);
 
 #ifdef ENTRYNODES_PRIVATE
 /**

src/or/networkstatus.c

@@ -1209,7 +1209,7 @@ should_delay_dir_fetches(const or_options_t *options, const char **msg_out)
   }
 
   if (options->UseBridges) {
-    if (!any_bridge_descriptors_known()) {
+    if (num_bridges_usable() == 0) {
       if (msg_out) {
         *msg_out = "No running bridges";
       }

src/test/test_dir.c

@@ -5875,11 +5875,11 @@ mock_networkstatus_consensus_can_use_extra_fallbacks(
   return mock_networkstatus_consensus_can_use_extra_fallbacks_value;
 }
 
-static int mock_any_bridge_descriptors_known_value = 0;
+static int mock_num_bridges_usable_value = 0;
 static int
-mock_any_bridge_descriptors_known(void)
+mock_num_bridges_usable(void)
 {
-  return mock_any_bridge_descriptors_known_value;
+  return mock_num_bridges_usable_value;
 }
 
 /* data is a 3 character nul-terminated string.
@@ -5908,17 +5908,18 @@ test_dir_find_dl_schedule(void* data)
   }
 
   if (str[2] == 'r') {
-    mock_any_bridge_descriptors_known_value = 1;
+    /* Any positive, non-zero value should work */
+    mock_num_bridges_usable_value = 2;
   } else {
-    mock_any_bridge_descriptors_known_value = 0;
+    mock_num_bridges_usable_value = 0;
   }
 
   MOCK(networkstatus_consensus_is_bootstrapping,
        mock_networkstatus_consensus_is_bootstrapping);
   MOCK(networkstatus_consensus_can_use_extra_fallbacks,
        mock_networkstatus_consensus_can_use_extra_fallbacks);
-  MOCK(any_bridge_descriptors_known,
+  MOCK(num_bridges_usable,
-       mock_any_bridge_descriptors_known);
+       mock_num_bridges_usable);
 
   download_status_t dls;
   smartlist_t server, client, server_cons, client_cons;
@@ -6030,7 +6031,7 @@ test_dir_find_dl_schedule(void* data)
   /* client */
   mock_options->ClientOnly = 1;
   mock_options->UseBridges = 1;
-  if (any_bridge_descriptors_known()) {
+  if (num_bridges_usable() > 0) {
     tt_ptr_op(find_dl_schedule(&dls, mock_options), OP_EQ, &bridge);
   } else {
     tt_ptr_op(find_dl_schedule(&dls, mock_options), OP_EQ, &bridge_bootstrap);
@@ -6039,7 +6040,7 @@ test_dir_find_dl_schedule(void* data)
  done:
   UNMOCK(networkstatus_consensus_is_bootstrapping);
   UNMOCK(networkstatus_consensus_can_use_extra_fallbacks);
-  UNMOCK(any_bridge_descriptors_known);
+  UNMOCK(num_bridges_usable);
   UNMOCK(get_options);
   tor_free(mock_options);
   mock_options = NULL;