Preserve bridge download status across SETCONF, HUP

This code changes it so that we don't remove bridges immediately when
we start re-parsing our configuration.  Instead, we mark them all, and
remove all the marked ones after re-parsing our bridge lines.  As we
add a bridge, we see if it's already in the list.  If so, we just
unmark it.

This new behavior will lose the property we used to have that bridges
were in bridge_list in the same order in which they appeared in the
torrc.  I took a quick look through the code, and I'm pretty sure we
didn't actually depend on that anywhere.

This is for bug 3019; it's a fix on 0.2.0.3-alpha.

4 files changed, 61 insertions, 12 deletions

diff --git a/changes/bug3019 b/changes/bug3019
new file mode 100644
index 0000000000..4df709fb3b
--- /dev/null
+++ b/changes/bug3019
@@ -0,0 +1,4 @@
+  o Minor bugfixes:
+    - Do not reset the bridge descriptor download status every time we
+      re-parse our configuration or get a configuration change.  Fixes
+      bug 3019; bugfix on Tor 0.2.0.3-alpha.
diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c
index a9986d309c..860cd27567 100644
--- a/src/or/circuitbuild.c
+++ b/src/or/circuitbuild.c
@@ -4471,6 +4471,9 @@ typedef struct {
   tor_addr_t addr;
   /** TLS port for the bridge. */
   uint16_t port;
+  /** Boolean: We are re-parsing our bridge list, and we are going to remove
+   * this one if we don't find it in the list of configured bridges. */
+  unsigned marked_for_removal : 1;
   /** Expected identity digest, or all zero bytes if we don't know what the
    * digest should be. */
   char identity[DIGEST_LEN];
@@ -4479,11 +4482,39 @@ typedef struct {
 } bridge_info_t;
 
 /** A list of configured bridges. Whenever we actually get a descriptor
- * for one, we add it as an entry guard. */
+ * for one, we add it as an entry guard.  Note that the order of bridges
+ * in this list does not necessarily correspond to the order of bridges
+ * in the torrc. */
 static smartlist_t *bridge_list = NULL;
 
-/** Initialize the bridge list to empty, creating it if needed. */
+/** Mark every entry of the bridge list to be removed on our next call to
+ * sweep_bridge_list unless it has first been un-marked. */
+void
+mark_bridge_list(void)
+{
+  if (!bridge_list)
+    bridge_list = smartlist_create();
+  SMARTLIST_FOREACH(bridge_list, bridge_info_t *, b,
+                    b->marked_for_removal = 1);
+}
+
+/** Remove every entry of the bridge list that was marked with
+ * mark_bridge_list if it has not subsequently been un-marked. */
 void
+sweep_bridge_list(void)
+{
+  if (!bridge_list)
+    bridge_list = smartlist_create();
+  SMARTLIST_FOREACH_BEGIN(bridge_list, bridge_info_t *, b) {
+    if (b->marked_for_removal) {
+      SMARTLIST_DEL_CURRENT(bridge_list, b);
+      tor_free(b);
+    }
+  } SMARTLIST_FOREACH_END(b);
+}
+
+/** Initialize the bridge list to empty, creating it if needed. */
+static void
 clear_bridge_list(void)
 {
   if (!bridge_list)
@@ -4496,7 +4527,8 @@ clear_bridge_list(void)
  * (either by comparing keys if possible, else by comparing addr/port).
  * Else return NULL. */
 static bridge_info_t *
-get_configured_bridge_by_addr_port_digest(tor_addr_t *addr, uint16_t port,
+get_configured_bridge_by_addr_port_digest(const tor_addr_t *addr,
+                                          uint16_t port,
                                           const char *digest)
 {
   if (!bridge_list)
@@ -4537,7 +4569,8 @@ routerinfo_is_a_configured_bridge(routerinfo_t *ri)
  * If it was a bridge, and we still don't know its digest, record it.
  */
 void
-learned_router_identity(tor_addr_t *addr, uint16_t port, const char *digest)
+learned_router_identity(const tor_addr_t *addr, uint16_t port,
+                        const char *digest)
 {
   bridge_info_t *bridge =
     get_configured_bridge_by_addr_port_digest(addr, port, digest);
@@ -4549,11 +4582,20 @@ learned_router_identity(tor_addr_t *addr, uint16_t port, const char *digest)
 }
 
 /** Remember a new bridge at <b>addr</b>:<b>port</b>. If <b>digest</b>
- * is set, it tells us the identity key too. */
+ * is set, it tells us the identity key too.  If we already had the
+ * bridge in our list, unmark it, and don't actually add anything new. */
 void
-bridge_add_from_config(const tor_addr_t *addr, uint16_t port, char *digest)
+bridge_add_from_config(const tor_addr_t *addr, uint16_t port,
+                       const char *digest)
 {
-  bridge_info_t *b = tor_malloc_zero(sizeof(bridge_info_t));
+  bridge_info_t *b;
+
+  if ((b = get_configured_bridge_by_addr_port_digest(addr, port, digest))) {
+    b->marked_for_removal = 0;
+    return;
+  }
+
+  b = tor_malloc_zero(sizeof(bridge_info_t));
   tor_addr_copy(&b->addr, addr);
   b->port = port;
   if (digest)
@@ -4561,6 +4603,7 @@ bridge_add_from_config(const tor_addr_t *addr, uint16_t port, char *digest)
   b->fetch_status.schedule = DL_SCHED_BRIDGE;
   if (!bridge_list)
     bridge_list = smartlist_create();
+
   smartlist_add(bridge_list, b);
 }
 
diff --git a/src/or/circuitbuild.h b/src/or/circuitbuild.h
index af24931878..0e673e1c05 100644
--- a/src/or/circuitbuild.h
+++ b/src/or/circuitbuild.h
@@ -62,12 +62,13 @@ int getinfo_helper_entry_guards(control_connection_t *conn,
                                 const char *question, char **answer,
                                 const char **errmsg);
 
-void clear_bridge_list(void);
+void mark_bridge_list(void);
+void sweep_bridge_list(void);
 int routerinfo_is_a_configured_bridge(routerinfo_t *ri);
-void
-learned_router_identity(tor_addr_t *addr, uint16_t port, const char *digest);
+void learned_router_identity(const tor_addr_t *addr, uint16_t port,
+                             const char *digest);
 void bridge_add_from_config(const tor_addr_t *addr, uint16_t port,
-                            char *digest);
+                            const char *digest);
 void retry_bridge_descriptor_fetch_directly(const char *digest);
 void fetch_bridge_descriptors(or_options_t *options, time_t now);
 void learned_bridge_descriptor(routerinfo_t *ri, int from_cache);
diff --git a/src/or/config.c b/src/or/config.c
index 0997fe2697..6a2742d954 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -1172,7 +1172,7 @@ options_act(or_options_t *old_options)
     return -1;
 
   if (options->Bridges) {
-    clear_bridge_list();
+    mark_bridge_list();
     for (cl = options->Bridges; cl; cl = cl->next) {
       if (parse_bridge_line(cl->value, 0)<0) {
         log_warn(LD_BUG,
@@ -1180,6 +1180,7 @@ options_act(or_options_t *old_options)
         return -1;
       }
     }
+    sweep_bridge_list();
   }
 
   if (running_tor && rend_config_services(options, 0)<0) {