[tor-commits] [tor/master] Stop setting bridges running in networkstatus_getinfo_by_purpose()

[nickm at torproject.org]

commit d4d77b277e72c74a47bd724531426d7f561607e4
Author: Neel Chauhan <neel at neelc.org>
Date:   Wed Apr 3 11:36:52 2019 -0400

    Stop setting bridges running in networkstatus_getinfo_by_purpose()
---
 changes/bug24490                         |  5 +++++
 scripts/maint/practracker/exceptions.txt |  6 +++---
 src/core/mainloop/mainloop.c             | 25 +++++++++++++++++++++++++
 src/feature/nodelist/networkstatus.c     |  4 ----
 4 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/changes/bug24490 b/changes/bug24490
new file mode 100644
index 000000000..9ae09dbd1
--- /dev/null
+++ b/changes/bug24490
@@ -0,0 +1,5 @@
+  o Minor bugfixes (bridge authority):
+    - We set bridges as running in a callback which runs every 5 minutes.
+      Previously, we set bridges as running in a GETINFO controller as
+      these shouldn't modify vital data structures. Fixes bug 24490;
+      bugfix on 0.2.0.13-alpha. Patch by Neel Chauhan
diff --git a/scripts/maint/practracker/exceptions.txt b/scripts/maint/practracker/exceptions.txt
index ad5d3e972..a9fcf9095 100644
--- a/scripts/maint/practracker/exceptions.txt
+++ b/scripts/maint/practracker/exceptions.txt
@@ -67,11 +67,11 @@ problem function-size /src/core/mainloop/connection.c:connection_handle_read_imp
 problem function-size /src/core/mainloop/connection.c:connection_buf_read_from_socket() 177
 problem function-size /src/core/mainloop/connection.c:connection_handle_write_impl() 241
 problem function-size /src/core/mainloop/connection.c:assert_connection_ok() 143
-problem file-size /src/core/mainloop/mainloop.c 3051
-problem include-count /src/core/mainloop/mainloop.c 66
+problem file-size /src/core/mainloop/mainloop.c 3076
+problem include-count /src/core/mainloop/mainloop.c 68
 problem function-size /src/core/mainloop/mainloop.c:conn_close_if_marked() 108
 problem function-size /src/core/mainloop/mainloop.c:run_connection_housekeeping() 123
-problem function-size /src/core/mainloop/mainloop.c:CALLBACK() 116
+problem function-size /src/core/mainloop/mainloop.c:CALLBACK() 118
 problem file-size /src/core/or/channel.c 3476
 problem function-size /src/core/or/channeltls.c:channel_tls_handle_var_cell() 160
 problem function-size /src/core/or/channeltls.c:channel_tls_process_versions_cell() 170
diff --git a/src/core/mainloop/mainloop.c b/src/core/mainloop/mainloop.c
index c9f2b0d89..800304a73 100644
--- a/src/core/mainloop/mainloop.c
+++ b/src/core/mainloop/mainloop.c
@@ -76,6 +76,7 @@
 #include "feature/control/control_events.h"
 #include "feature/dirauth/authmode.h"
 #include "feature/dirauth/reachability.h"
+#include "feature/dirauth/voteflags.h"
 #include "feature/dircache/consdiffmgr.h"
 #include "feature/dircache/dirserv.h"
 #include "feature/dircommon/directory.h"
@@ -87,6 +88,7 @@
 #include "feature/nodelist/networkstatus.h"
 #include "feature/nodelist/nodelist.h"
 #include "feature/nodelist/routerlist.h"
+#include "feature/nodelist/routerlist_st.h"
 #include "feature/relay/dns.h"
 #include "feature/relay/routerkeys.h"
 #include "feature/relay/routermode.h"
@@ -1375,6 +1377,7 @@ CALLBACK(rotate_onion_key);
 CALLBACK(rotate_x509_certificate);
 CALLBACK(save_stability);
 CALLBACK(save_state);
+CALLBACK(set_bridge_running);
 CALLBACK(write_bridge_ns);
 CALLBACK(write_stats_file);
 CALLBACK(control_per_second_events);
@@ -1453,6 +1456,7 @@ STATIC periodic_event_item_t periodic_events[] = {
 
   /* Bridge Authority only. */
   CALLBACK(write_bridge_ns, BRIDGEAUTH, 0),
+  CALLBACK(set_bridge_running, BRIDGEAUTH, 0),
 
   /* Directory server only. */
   CALLBACK(clean_consdiffmgr, DIRSERVER, 0),
@@ -2583,6 +2587,27 @@ write_bridge_ns_callback(time_t now, const or_options_t *options)
   return PERIODIC_EVENT_NO_UPDATE;
 }
 
+/**
+ * Periodic callback: if we're the bridge authority, set the running flag on
+ * bridges if they're reachable
+ */
+static int
+set_bridge_running_callback(time_t now, const or_options_t *options)
+{
+  if (options->BridgeAuthoritativeDir) {
+    routerlist_t *rl = router_get_routerlist();
+
+    SMARTLIST_FOREACH_BEGIN(rl->routers, routerinfo_t *, ri) {
+      if (ri->purpose == ROUTER_PURPOSE_BRIDGE)
+        dirserv_set_router_is_running(ri, now);
+    } SMARTLIST_FOREACH_END(ri);
+
+#define SET_BRIDGES_RUNNING_INTERVAL (3*60)
+    return SET_BRIDGES_RUNNING_INTERVAL;
+  }
+  return PERIODIC_EVENT_NO_UPDATE;
+}
+
 static int heartbeat_callback_first_time = 1;
 
 /**
diff --git a/src/feature/nodelist/networkstatus.c b/src/feature/nodelist/networkstatus.c
index ea9f12367..bc12fa407 100644
--- a/src/feature/nodelist/networkstatus.c
+++ b/src/feature/nodelist/networkstatus.c
@@ -2378,7 +2378,6 @@ networkstatus_getinfo_by_purpose(const char *purpose_string, time_t now)
   smartlist_t *statuses;
   const uint8_t purpose = router_purpose_from_string(purpose_string);
   routerstatus_t rs;
-  const int bridge_auth = authdir_mode_bridge(get_options());
 
   if (purpose == ROUTER_PURPOSE_UNKNOWN) {
     log_info(LD_DIR, "Unrecognized purpose '%s' when listing router statuses.",
@@ -2395,9 +2394,6 @@ networkstatus_getinfo_by_purpose(const char *purpose_string, time_t now)
       continue;
     if (ri->purpose != purpose)
       continue;
-    /* TODO: modifying the running flag in a getinfo is a bad idea */
-    if (bridge_auth && ri->purpose == ROUTER_PURPOSE_BRIDGE)
-      dirserv_set_router_is_running(ri, now);
     /* then generate and write out status lines for each of them */
     set_routerstatus_from_routerinfo(&rs, node, ri, now, 0);
     smartlist_add(statuses, networkstatus_getinfo_helper_single(&rs));