Only update guard status (usable / not usable) once we have

enough directory information. This was causing us to always pick
two new guards on startup (bugfix on 0.2.0.9-alpha), and it was
causing us to discard all our guards on startup if we hadn't been
running for a few weeks (bugfix on 0.1.2.x). Fixes bug 448.


svn:r12570

5 files changed, 37 insertions, 18 deletions

diff --git a/ChangeLog b/ChangeLog
index ece9aaf78..9869479e5 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,4 +1,11 @@
 Changes in version 0.2.0.13-alpha - 2007-11-??
+  o Major bugfixes:
+    - Only update guard status (usable / not usable) once we have
+      enough directory information. This was causing us to always pick
+      two new guards on startup (bugfix on 0.2.0.9-alpha), and it was
+      causing us to discard all our guards on startup if we hadn't been
+      running for a few weeks (bugfix on 0.1.2.x). Fixes bug 448.
+
   o Minor bugfixes:
     - The fix in 0.2.0.12-alpha cleared the "hsdir" flag in v3 network
       consensus documents when there are too many relays at a single
diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c
index 6b159ec6f..995978b57 100644
--- a/src/or/circuitbuild.c
+++ b/src/or/circuitbuild.c
@@ -1844,40 +1844,44 @@ build_state_get_exit_nickname(cpath_build_state_t *state)
 /** Check whether the entry guard <b>e</b> is usable, given the directory
  * authorities' opinion about the router (stored in <b>ri</b>) and the user's
  * configuration (in <b>options</b>). Set <b>e</b>-&gt;bad_since
- * accordingly. Return true iff the entry guard's status changes. */
+ * accordingly. Return true iff the entry guard's status changes.
+ *
+ * If it's not usable, set *<b>reason</b> to a static string explaining why.
+ */
 static int
 entry_guard_set_status(entry_guard_t *e, routerinfo_t *ri,
-                       time_t now, or_options_t *options)
+                       time_t now, or_options_t *options, const char **reason)
 {
-  const char *reason = NULL;
   char buf[HEX_DIGEST_LEN+1];
   int changed = 0;
 
   tor_assert(options);
 
+  *reason = NULL;
+
   /* Do we want to mark this guard as bad? */
   if (!ri)
-    reason = "unlisted";
+    *reason = "unlisted";
   else if (!ri->is_running)
-    reason = "down";
+    *reason = "down";
   else if (options->UseBridges && ri->purpose != ROUTER_PURPOSE_BRIDGE)
-    reason = "not a bridge";
+    *reason = "not a bridge";
   else if (!options->UseBridges && !ri->is_possible_guard &&
            !router_nickname_is_in_list(ri, options->EntryNodes))
-    reason = "not recommended as a guard";
+    *reason = "not recommended as a guard";
   else if (router_nickname_is_in_list(ri, options->ExcludeNodes))
-    reason = "excluded";
+    *reason = "excluded";
 
-  if (reason && ! e->bad_since) {
+  if (*reason && ! e->bad_since) {
     /* Router is newly bad. */
     base16_encode(buf, sizeof(buf), e->identity, DIGEST_LEN);
     log_info(LD_CIRC, "Entry guard %s (%s) is %s: marking as unusable.",
-             e->nickname, buf, reason);
+             e->nickname, buf, *reason);
 
     e->bad_since = now;
     control_event_guard(e->nickname, e->identity, "BAD");
     changed = 1;
-  } else if (!reason && e->bad_since) {
+  } else if (!*reason && e->bad_since) {
     /* There's nothing wrong with the router any more. */
     base16_encode(buf, sizeof(buf), e->identity, DIGEST_LEN);
     log_info(LD_CIRC, "Entry guard %s (%s) is no longer unusable: "
@@ -2185,12 +2189,13 @@ remove_dead_entry_guards(void)
  *
  * An entry is 'down' if the directory lists it as nonrunning.
  * An entry is 'unlisted' if the directory doesn't include it.
+ *
+ * Don't call this on startup; only on a fresh download. Otherwise we'll
+ * think that things are unlisted.
  */
 void
 entry_guards_compute_status(void)
 {
-  /* Don't call this on startup; only on a fresh download.  Otherwise we'll
-   * think that things are unlisted. */
   time_t now;
   int changed = 0;
   int severity = LOG_INFO;
@@ -2205,13 +2210,18 @@ entry_guards_compute_status(void)
   SMARTLIST_FOREACH(entry_guards, entry_guard_t *, entry,
     {
       routerinfo_t *r = router_get_by_digest(entry->identity);
-      if (entry_guard_set_status(entry, r, now, options))
+      const char *reason = NULL;
+      if (entry_guard_set_status(entry, r, now, options, &reason))
         changed = 1;
 
-      log_info(LD_CIRC, "Summary: Entry '%s' is %s, %s and %s.",
+      if (entry->bad_since)
+        tor_assert(reason);
+
+      log_info(LD_CIRC, "Summary: Entry '%s' is %s, %s%s, and %s.",
                entry->nickname,
                entry->unreachable_since ? "unreachable" : "reachable",
-               entry->bad_since ? "unusable" : "usable",
+               entry->bad_since ? "unusable: " : "usable",
+               entry->bad_since ? reason : "",
                entry_is_live(entry, 0, 1, 0) ? "live" : "not live");
     });
 
diff --git a/src/or/directory.c b/src/or/directory.c
index 34162ce6b..4743f9578 100644
--- a/src/or/directory.c
+++ b/src/or/directory.c
@@ -1181,7 +1181,7 @@ load_downloaded_routers(const char *body, smartlist_t *which,
 }
 
 /** We are a client, and we've finished reading the server's
- * response. Parse and it and act appropriately.
+ * response. Parse it and act appropriately.
  *
  * If we're still happy with using this directory server in the future, return
  * 0. Otherwise return -1; and the caller should consider trying the request
diff --git a/src/or/main.c b/src/or/main.c
index 775c5f656..9d88b7621 100644
--- a/src/or/main.c
+++ b/src/or/main.c
@@ -663,6 +663,9 @@ directory_info_has_arrived(time_t now, int from_cache)
     update_router_descriptor_downloads(now);
     return;
   } else {
+    /* if we have enough dir info, then update our guard status with
+     * whatever we just learned. */
+    entry_guards_compute_status();
     /* Don't even bother trying to get extrainfo until the rest of our
      * directory info is up-to-date */
     if (options->DownloadExtraInfo)
diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c
index 150a5d1b8..3ede96eaf 100644
--- a/src/or/networkstatus.c
+++ b/src/or/networkstatus.c
@@ -1423,7 +1423,6 @@ routers_update_all_from_networkstatus(time_t now, int dir_version)
                     ri->cache_info.routerlist_index = ri_sl_idx);
   if (rl->old_routers)
     signed_descs_update_status_from_consensus_networkstatus(rl->old_routers);
-  entry_guards_compute_status();
 
   me = router_get_my_routerinfo();
   if (me && !have_warned_about_invalid_status) {