Merge branch 'bug911'

changes/bug911_hibernate_precludes_Running

@@ -0,0 +1,6 @@
+  o Minor bugfixes:
+    - Never vote for a server as "Running" if we have a descriptor for it
+      claiming to be hibernating, and that descriptor was published more
+      recently than our last contact with the server.  Bugfix on
+      0.2.0.3-alpha; fixes bug 911.
+

src/or/dirserv.c

@@ -730,6 +730,10 @@ dirserv_add_descriptor(routerinfo_t *ri, const char **msg, const char *source)
   desc = tor_strndup(ri->cache_info.signed_descriptor_body, desclen);
   nickname = tor_strdup(ri->nickname);
 
+  /* Tell if we're about to need to launch a test if we add this. */
+  ri->needs_retest_if_added =
+    dirserv_should_launch_reachability_test(ri, ri_old);
+
   r = router_add_to_routerlist(ri, msg, 0, 0);
   if (!WRA_WAS_ADDED(r)) {
     /* unless the routerinfo was fine, just out-of-date */
@@ -744,7 +748,7 @@ dirserv_add_descriptor(routerinfo_t *ri, const char **msg, const char *source)
 
     changed = smartlist_create();
     smartlist_add(changed, ri);
-    control_event_descriptors_changed(changed);
+    routerlist_descriptors_added(changed, 0);
     smartlist_free(changed);
     if (!*msg) {
       *msg =  ri->is_valid ? "Descriptor for valid server accepted" :
@@ -923,6 +927,11 @@ running_long_enough_to_decide_unreachable(void)
  * the directory. */
 #define REACHABLE_TIMEOUT (45*60)
 
+/** If we tested a router and found it reachable _at least this long_ after it
+ * declared itself hibernating, it is probably done hibernating and we just
+ * missed a descriptor from it. */
+#define HIBERNATION_PUBLICATION_SKEW (60*60)
+
 /** Treat a router as alive if
  *    - It's me, and I'm not hibernating.
  * or - We've found it reachable recently. */
@@ -935,11 +944,23 @@ dirserv_set_router_is_running(routerinfo_t *router, time_t now)
    */
   int answer;
 
-  if (router_is_me(router) && !we_are_hibernating())
+  if (router_is_me(router)) {
+    /* We always know if we are down ourselves. */
+    answer = ! we_are_hibernating();
+  } else if (router->is_hibernating &&
+             (router->cache_info.published_on +
+              HIBERNATION_PUBLICATION_SKEW) > router->last_reachable) {
+    /* A hibernating router is down unless we (somehow) had contact with it
+     * since it declared itself to be hibernating. */
+    answer = 0;
+  } else if (get_options()->AssumeReachable) {
+    /* If AssumeReachable, everybody is up unless they say they are down! */
     answer = 1;
-  else
-    answer = get_options()->AssumeReachable ||
-             now < router->last_reachable + REACHABLE_TIMEOUT;
+  } else {
+    /* Otherwise, a router counts as up if we found it reachable in the last
+       REACHABLE_TIMEOUT seconds. */
+    answer = (now < router->last_reachable + REACHABLE_TIMEOUT);
+  }
 
   if (!answer && running_long_enough_to_decide_unreachable()) {
     /* not considered reachable. tell rephist. */
@@ -3098,6 +3119,26 @@ dirserv_orconn_tls_done(const char *address,
    * skip testing. */
 }
 
+/** Called when we, as an authority, receive a new router descriptor either as
+ * an upload or a download.  Used to decide whether to relaunch reachability
+ * testing for the server. */
+int
+dirserv_should_launch_reachability_test(routerinfo_t *ri, routerinfo_t *ri_old)
+{
+  if (!authdir_mode_handles_descs(get_options(), ri->purpose))
+    return 0;
+  if (!ri_old) {
+    /* New router: Launch an immediate reachability test, so we will have an
+     * opinion soon in case we're generating a consensus soon */
+    return 1;
+  }
+  if (ri_old->is_hibernating && !ri->is_hibernating) {
+    /* It just came out of hibernation; launch a reachability test */
+    return 1;
+  }
+  return 0;
+}
+
 /** Helper function for dirserv_test_reachability(). Start a TLS
  * connection to <b>router</b>, and annotate it with when we started
  * the test. */

src/or/dirserv.h

@@ -99,6 +99,8 @@ void dirserv_orconn_tls_done(const char *address,
                              uint16_t or_port,
                              const char *digest_rcvd,
                              int as_advertised);
+int dirserv_should_launch_reachability_test(routerinfo_t *ri,
+                                            routerinfo_t *ri_old);
 void dirserv_single_reachability_test(time_t now, routerinfo_t *router);
 void dirserv_test_reachability(time_t now);
 int authdir_wants_to_reject_router(routerinfo_t *ri, const char **msg,

src/or/networkstatus.c

@@ -1981,6 +1981,15 @@ routers_update_status_from_consensus_networkstatus(smartlist_t *routers,
       router->is_bad_directory = rs->is_bad_directory;
       router->is_bad_exit = rs->is_bad_exit;
       router->is_hs_dir = rs->is_hs_dir;
+    } else {
+      /* If we _are_ an authority, we should check whether this router
+       * is one that will cause us to need a reachability test. */
+      routerinfo_t *old_router =
+        router_get_by_digest(router->cache_info.identity_digest);
+      if (old_router != router) {
+        router->needs_retest_if_added =
+          dirserv_should_launch_reachability_test(router, old_router);
+      }
     }
     if (router->is_running && ds) {
       download_status_reset(&ds->v2_ns_dl_status);

src/or/or.h

@@ -1456,6 +1456,9 @@ typedef struct {
                              * directory according to the authorities. */
   unsigned int policy_is_reject_star:1; /**< True iff the exit policy for this
                                          * router rejects everything. */
+  /** True if, after we have added this router, we should re-launch
+   * tests for it. */
+  unsigned int needs_retest_if_added:1;
 
 /** Tor can use this router for general positions in circuits. */
 #define ROUTER_PURPOSE_GENERAL 0

src/or/routerlist.c

@@ -3300,11 +3300,6 @@ router_add_to_routerlist(routerinfo_t *router, const char **msg,
    * the list. */
   routerlist_insert(routerlist, router);
   if (!from_cache) {
-    if (authdir) {
-      /* launch an immediate reachability test, so we will have an opinion
-       * soon in case we're generating a consensus soon */
-      dirserv_single_reachability_test(time(NULL), router);
-    }
     signed_desc_append_to_journal(&router->cache_info,
                                   &routerlist->desc_store);
   }
@@ -3624,15 +3619,19 @@ routerlist_remove_old_routers(void)
 
 /** We just added a new set of descriptors. Take whatever extra steps
  * we need. */
-static void
+void
 routerlist_descriptors_added(smartlist_t *sl, int from_cache)
 {
   tor_assert(sl);
   control_event_descriptors_changed(sl);
-  SMARTLIST_FOREACH(sl, routerinfo_t *, ri,
+  SMARTLIST_FOREACH_BEGIN(sl, routerinfo_t *, ri) {
     if (ri->purpose == ROUTER_PURPOSE_BRIDGE)
       learned_bridge_descriptor(ri, from_cache);
-  );
+    if (ri->needs_retest_if_added) {
+      ri->needs_retest_if_added = 0;
+      dirserv_single_reachability_test(approx_time(), ri);
+    }
+  } SMARTLIST_FOREACH_END(ri);
 }
 
 /**

src/or/routerlist.h

@@ -116,6 +116,7 @@ was_router_added_t router_add_to_routerlist(routerinfo_t *router,
 was_router_added_t router_add_extrainfo_to_routerlist(
                                         extrainfo_t *ei, const char **msg,
                                         int from_cache, int from_fetch);
+void routerlist_descriptors_added(smartlist_t *sl, int from_cache);
 void routerlist_remove_old_routers(void);
 int router_load_single_router(const char *s, uint8_t purpose, int cache,
                               const char **msg);