Merge branch 'tor-github/pr/993'

changes/ticket30293

@@ -0,0 +1,5 @@
+  o Code simplification and refactoring:
+    - Start move responsibility for knowing about periodic events to the
+      appropriate subsystems, so that the mainloop doesn't need to know all
+      the periodic events in the rest of the codebase.  Implements tickets
+      30293 and 30294.

scripts/maint/practracker/exceptions.txt

@@ -29,8 +29,8 @@
 #
 # Remember: It is better to fix the problem than to add a new exception!
 
-problem file-size /src/app/config/config.c 8491
-problem include-count /src/app/config/config.c 86
+problem file-size /src/app/config/config.c 8492
+problem include-count /src/app/config/config.c 87
 problem function-size /src/app/config/config.c:options_act_reversible() 296
 problem function-size /src/app/config/config.c:options_act() 588
 problem function-size /src/app/config/config.c:resolve_my_address() 192
@@ -214,7 +214,7 @@ problem function-size /src/feature/nodelist/authcert.c:trusted_dirs_load_certs_f
 problem function-size /src/feature/nodelist/authcert.c:authority_certs_fetch_missing() 296
 problem function-size /src/feature/nodelist/fmt_routerstatus.c:routerstatus_format_entry() 166
 problem function-size /src/feature/nodelist/microdesc.c:microdesc_cache_rebuild() 134
-problem include-count /src/feature/nodelist/networkstatus.c 61
+problem include-count /src/feature/nodelist/networkstatus.c 62
 problem function-size /src/feature/nodelist/networkstatus.c:networkstatus_check_consensus_signature() 176
 problem function-size /src/feature/nodelist/networkstatus.c:networkstatus_set_current_consensus() 293
 problem function-size /src/feature/nodelist/node_select.c:router_pick_directory_server_impl() 123

src/app/config/config.c

@@ -156,6 +156,7 @@
 #include "lib/evloop/procmon.h"
 
 #include "feature/dirauth/dirvote.h"
+#include "feature/dirauth/dirauth_periodic.h"
 #include "feature/dirauth/recommend_pkg.h"
 #include "feature/dirauth/authmode.h"
 

src/app/main/shutdown.c

@@ -18,7 +18,6 @@
 #include "app/main/shutdown.h"
 #include "app/main/subsysmgr.h"
 #include "core/mainloop/connection.h"
-#include "core/mainloop/mainloop.h"
 #include "core/mainloop/mainloop_pubsub.h"
 #include "core/or/channeltls.h"
 #include "core/or/circuitlist.h"
@@ -126,7 +125,6 @@ tor_free_all(int postfork)
   }
   geoip_free_all();
   geoip_stats_free_all();
-  dirvote_free_all();
   routerlist_free_all();
   networkstatus_free_all();
   addressmap_free_all();
@@ -176,7 +174,6 @@ tor_free_all(int postfork)
   /* stuff in main.c */
 
   tor_mainloop_disconnect_pubsub();
-  tor_mainloop_free_all();
 
   if (!postfork) {
     release_lockfile();

src/app/main/subsystem_list.c

@@ -8,6 +8,7 @@
 #include "lib/cc/compat_compiler.h"
 #include "lib/cc/torint.h"
 
+#include "core/mainloop/mainloop_sys.h"
 #include "core/or/ocirc_event_sys.h"
 #include "core/or/orconn_event_sys.h"
 #include "feature/control/btrack_sys.h"
@@ -23,6 +24,8 @@
 #include "lib/wallclock/wallclock_sys.h"
 #include "lib/process/process_sys.h"
 
+#include "feature/dirauth/dirauth_sys.h"
+
 #include <stddef.h>
 
 /**
@@ -44,6 +47,12 @@ const subsys_fns_t *tor_subsystems[] = {
   &sys_orconn_event, /* -33 */
   &sys_ocirc_event, /* -32 */
   &sys_btrack, /* -30 */
+
+  &sys_mainloop, /* 5 */
+
+#ifdef HAVE_MODULE_DIRAUTH
+  &sys_dirauth, /* 70 */
+#endif
 };
 
 const unsigned n_tor_subsystems = ARRAY_LENGTH(tor_subsystems);

src/core/include.am

@@ -24,6 +24,7 @@ LIBTOR_APP_A_SOURCES = 				\
 	src/core/mainloop/cpuworker.c		\
 	src/core/mainloop/mainloop.c		\
 	src/core/mainloop/mainloop_pubsub.c	\
+	src/core/mainloop/mainloop_sys.c	\
 	src/core/mainloop/netstatus.c		\
 	src/core/mainloop/periodic.c		\
 	src/core/or/address_set.c		\
@@ -175,6 +176,8 @@ LIBTOR_APP_TESTING_A_SOURCES = $(LIBTOR_APP_A_SOURCES)
 # The Directory Authority module.
 MODULE_DIRAUTH_SOURCES = 					\
 	src/feature/dirauth/authmode.c				\
+	src/feature/dirauth/dirauth_periodic.c			\
+	src/feature/dirauth/dirauth_sys.c			\
 	src/feature/dirauth/dircollate.c			\
 	src/feature/dirauth/dirvote.c				\
 	src/feature/dirauth/shared_random.c			\
@@ -222,6 +225,7 @@ noinst_HEADERS +=					\
 	src/core/mainloop/cpuworker.h			\
 	src/core/mainloop/mainloop.h			\
 	src/core/mainloop/mainloop_pubsub.h		\
+	src/core/mainloop/mainloop_sys.h	        \
 	src/core/mainloop/netstatus.h			\
 	src/core/mainloop/periodic.h			\
 	src/core/or/addr_policy_st.h			\
@@ -307,6 +311,8 @@ noinst_HEADERS +=					\
 	src/feature/control/getinfo_geoip.h		\
 	src/feature/dirauth/authmode.h			\
 	src/feature/dirauth/bwauth.h			\
+	src/feature/dirauth/dirauth_periodic.h		\
+	src/feature/dirauth/dirauth_sys.h		\
 	src/feature/dirauth/dircollate.h		\
 	src/feature/dirauth/dirvote.h			\
 	src/feature/dirauth/dsigs_parse.h		\

src/core/mainloop/mainloop.c

@@ -75,7 +75,6 @@
 #include "feature/control/control.h"
 #include "feature/control/control_events.h"
 #include "feature/dirauth/authmode.h"
-#include "feature/dirauth/reachability.h"
 #include "feature/dircache/consdiffmgr.h"
 #include "feature/dircache/dirserv.h"
 #include "feature/dircommon/directory.h"
@@ -106,9 +105,6 @@
 
 #include <event2/event.h>
 
-#include "feature/dirauth/dirvote.h"
-#include "feature/dirauth/authmode.h"
-
 #include "core/or/cell_st.h"
 #include "core/or/entry_connection_st.h"
 #include "feature/nodelist/networkstatus_st.h"
@@ -1346,7 +1342,6 @@ static int periodic_events_initialized = 0;
 #define CALLBACK(name) \
   static int name ## _callback(time_t, const or_options_t *)
 CALLBACK(add_entropy);
-CALLBACK(check_authority_cert);
 CALLBACK(check_canonical_channels);
 CALLBACK(check_descriptor);
 CALLBACK(check_dns_honesty);
@@ -1356,14 +1351,11 @@ CALLBACK(check_for_reachability_bw);
 CALLBACK(check_onion_keys_expiry_time);
 CALLBACK(clean_caches);
 CALLBACK(clean_consdiffmgr);
-CALLBACK(dirvote);
-CALLBACK(downrate_stability);
 CALLBACK(expire_old_ciruits_serverside);
 CALLBACK(fetch_networkstatus);
 CALLBACK(heartbeat);
 CALLBACK(hs_service);
 CALLBACK(launch_descriptor_fetches);
-CALLBACK(launch_reachability_tests);
 CALLBACK(prune_old_routers);
 CALLBACK(reachability_warnings);
 CALLBACK(record_bridge_stats);
@@ -1373,7 +1365,6 @@ CALLBACK(retry_dns);
 CALLBACK(retry_listeners);
 CALLBACK(rotate_onion_key);
 CALLBACK(rotate_x509_certificate);
-CALLBACK(save_stability);
 CALLBACK(save_state);
 CALLBACK(write_bridge_ns);
 CALLBACK(write_stats_file);
@@ -1387,7 +1378,7 @@ CALLBACK(second_elapsed);
   PERIODIC_EVENT(name, PERIODIC_EVENT_ROLE_ ## r, f)
 #define FL(name) (PERIODIC_EVENT_FLAG_ ## name)
 
-STATIC periodic_event_item_t periodic_events[] = {
+STATIC periodic_event_item_t mainloop_periodic_events[] = {
 
   /* Everyone needs to run these. They need to have very long timeouts for
    * that to be safe. */
@@ -1428,15 +1419,6 @@ STATIC periodic_event_item_t periodic_events[] = {
   CALLBACK(retry_dns, ROUTER, 0),
   CALLBACK(rotate_onion_key, ROUTER, 0),
 
-  /* Authorities (bridge and directory) only. */
-  CALLBACK(downrate_stability, AUTHORITIES, 0),
-  CALLBACK(launch_reachability_tests, AUTHORITIES, FL(NEED_NET)),
-  CALLBACK(save_stability, AUTHORITIES, 0),
-
-  /* Directory authority only. */
-  CALLBACK(check_authority_cert, DIRAUTH, 0),
-  CALLBACK(dirvote, DIRAUTH, FL(NEED_NET)),
-
   /* Relay only. */
   CALLBACK(check_canonical_channels, RELAY, FL(NEED_NET)),
   CALLBACK(check_dns_honesty, RELAY, FL(NEED_NET)),
@@ -1470,7 +1452,6 @@ STATIC periodic_event_item_t periodic_events[] = {
  * can access them by name.  We also keep them inside periodic_events[]
  * so that we can implement "reset all timers" in a reasonable way. */
 static periodic_event_item_t *check_descriptor_event=NULL;
-static periodic_event_item_t *dirvote_event=NULL;
 static periodic_event_item_t *fetch_networkstatus_event=NULL;
 static periodic_event_item_t *launch_descriptor_fetches_event=NULL;
 static periodic_event_item_t *check_dns_honesty_event=NULL;
@@ -1485,24 +1466,7 @@ static periodic_event_item_t *prune_old_routers_event=NULL;
 void
 reset_all_main_loop_timers(void)
 {
-  int i;
-  for (i = 0; periodic_events[i].name; ++i) {
-    periodic_event_reschedule(&periodic_events[i]);
-  }
-}
-
-/** Return the member of periodic_events[] whose name is <b>name</b>.
- * Return NULL if no such event is found.
- */
-static periodic_event_item_t *
-find_periodic_event(const char *name)
-{
-  int i;
-  for (i = 0; periodic_events[i].name; ++i) {
-    if (strcmp(name, periodic_events[i].name) == 0)
-      return &periodic_events[i];
-  }
-  return NULL;
+  periodic_events_reset_all();
 }
 
 /** Return a bitmask of the roles this tor instance is configured for using
@@ -1565,9 +1529,9 @@ initialize_periodic_events_cb(evutil_socket_t fd, short events, void *data)
   rescan_periodic_events(get_options());
 }
 
-/** Set up all the members of periodic_events[], and configure them all to be
- * launched from a callback. */
-STATIC void
+/** Set up all the members of mainloop_periodic_events[], and configure them
+ * all to be launched from a callback. */
+void
 initialize_periodic_events(void)
 {
   if (periodic_events_initialized)
@@ -1575,37 +1539,33 @@ initialize_periodic_events(void)
 
   periodic_events_initialized = 1;
 
-  /* Set up all periodic events. We'll launch them by roles. */
-  int i;
-  for (i = 0; periodic_events[i].name; ++i) {
-    periodic_event_setup(&periodic_events[i]);
+  for (int i = 0; mainloop_periodic_events[i].name; ++i) {
+    periodic_events_register(&mainloop_periodic_events[i]);
   }
 
+  /* Set up all periodic events. We'll launch them by roles. */
+
 #define NAMED_CALLBACK(name) \
-  STMT_BEGIN name ## _event = find_periodic_event( #name ); STMT_END
+  STMT_BEGIN name ## _event = periodic_events_find( #name ); STMT_END
 
   NAMED_CALLBACK(check_descriptor);
   NAMED_CALLBACK(prune_old_routers);
-  NAMED_CALLBACK(dirvote);
   NAMED_CALLBACK(fetch_networkstatus);
   NAMED_CALLBACK(launch_descriptor_fetches);
   NAMED_CALLBACK(check_dns_honesty);
   NAMED_CALLBACK(save_state);
-
-  struct timeval one_second = { 1, 0 };
-  initialize_periodic_events_event = tor_evtimer_new(
-                  tor_libevent_get_base(),
-                  initialize_periodic_events_cb, NULL);
-  event_add(initialize_periodic_events_event, &one_second);
 }
 
 STATIC void
 teardown_periodic_events(void)
 {
-  int i;
-  for (i = 0; periodic_events[i].name; ++i) {
-    periodic_event_destroy(&periodic_events[i]);
-  }
+  periodic_events_disconnect_all();
+  check_descriptor_event = NULL;
+  fetch_networkstatus_event = NULL;
+  launch_descriptor_fetches_event = NULL;
+  check_dns_honesty_event = NULL;
+  save_state_event = NULL;
+  prune_old_routers_event = NULL;
   periodic_events_initialized = 0;
 }
 
@@ -1640,40 +1600,7 @@ rescan_periodic_events(const or_options_t *options)
 {
   tor_assert(options);
 
-  /* Avoid scanning the event list if we haven't initialized it yet. This is
-   * particularly useful for unit tests in order to avoid initializing main
-   * loop events everytime. */
-  if (!periodic_events_initialized) {
-    return;
-  }
-
-  int roles = get_my_roles(options);
-
-  for (int i = 0; periodic_events[i].name; ++i) {
-    periodic_event_item_t *item = &periodic_events[i];
-
-    int enable = !!(item->roles & roles);
-
-    /* Handle the event flags. */
-    if (net_is_disabled() &&
-        (item->flags & PERIODIC_EVENT_FLAG_NEED_NET)) {
-      enable = 0;
-    }
-
-    /* Enable the event if needed. It is safe to enable an event that was
-     * already enabled. Same goes for disabling it. */
-    if (enable) {
-      log_debug(LD_GENERAL, "Launching periodic event %s", item->name);
-      periodic_event_enable(item);
-    } else {
-      log_debug(LD_GENERAL, "Disabling periodic event %s", item->name);
-      if (item->flags & PERIODIC_EVENT_FLAG_RUN_ON_DISABLE) {
-        periodic_event_schedule_and_disable(item);
-      } else {
-        periodic_event_disable(item);
-      }
-    }
-  }
+  periodic_events_rescan_by_roles(get_my_roles(options), net_is_disabled());
 }
 
 /* We just got new options globally set, see if we need to enabled or disable
@@ -1681,13 +1608,7 @@ rescan_periodic_events(const or_options_t *options)
 void
 periodic_events_on_new_options(const or_options_t *options)
 {
-  /* Only if we've already initialized the events, rescan the list which will
-   * enable or disable events depending on our roles. This will be called at
-   * bootup and we don't want this function to initialize the events because
-   * they aren't set up at this stage. */
-  if (periodic_events_initialized) {
-    rescan_periodic_events(options);
-  }
+  rescan_periodic_events(options);
 }
 
 /**
@@ -1770,29 +1691,6 @@ mainloop_schedule_shutdown(int delay_sec)
   mainloop_event_schedule(scheduled_shutdown_ev, &delay_tv);
 }
 
-#define LONGEST_TIMER_PERIOD (30 * 86400)
-/** Helper: Return the number of seconds between <b>now</b> and <b>next</b>,
- * clipped to the range [1 second, LONGEST_TIMER_PERIOD]. */
-static inline int
-safe_timer_diff(time_t now, time_t next)
-{
-  if (next > now) {
-    /* There were no computers at signed TIME_MIN (1902 on 32-bit systems),
-     * and nothing that could run Tor. It's a bug if 'next' is around then.
-     * On 64-bit systems with signed TIME_MIN, TIME_MIN is before the Big
-     * Bang. We cannot extrapolate past a singularity, but there was probably
-     * nothing that could run Tor then, either.
-     **/
-    tor_assert(next > TIME_MIN + LONGEST_TIMER_PERIOD);
-
-    if (next - LONGEST_TIMER_PERIOD > now)
-      return LONGEST_TIMER_PERIOD;
-    return (int)(next - now);
-  } else {
-    return 1;
-  }
-}
-
 /** Perform regular maintenance tasks.  This function gets run once per
  * second.
  */
@@ -2061,102 +1959,6 @@ check_network_participation_callback(time_t now, const or_options_t *options)
   return CHECK_PARTICIPATION_INTERVAL;
 }
 
-/**
- * Periodic callback: if we're an authority, make sure we test
- * the routers on the network for reachability.
- */
-static int
-launch_reachability_tests_callback(time_t now, const or_options_t *options)
-{
-  if (authdir_mode_tests_reachability(options) &&
-      !net_is_disabled()) {
-    /* try to determine reachability of the other Tor relays */
-    dirserv_test_reachability(now);
-  }
-  return REACHABILITY_TEST_INTERVAL;
-}
-
-/**
- * Periodic callback: if we're an authority, discount the stability
- * information (and other rephist information) that's older.
- */
-static int
-downrate_stability_callback(time_t now, const or_options_t *options)
-{
-  (void)options;
-  /* 1d. Periodically, we discount older stability information so that new
-   * stability info counts more, and save the stability information to disk as
-   * appropriate. */
-  time_t next = rep_hist_downrate_old_runs(now);
-  return safe_timer_diff(now, next);
-}
-
-/**
- * Periodic callback: if we're an authority, record our measured stability
- * information from rephist in an mtbf file.
- */
-static int
-save_stability_callback(time_t now, const or_options_t *options)
-{
-  if (authdir_mode_tests_reachability(options)) {
-    if (rep_hist_record_mtbf_data(now, 1)<0) {
-      log_warn(LD_GENERAL, "Couldn't store mtbf data.");
-    }
-  }
-#define SAVE_STABILITY_INTERVAL (30*60)
-  return SAVE_STABILITY_INTERVAL;
-}
-
-/**
- * Periodic callback: if we're an authority, check on our authority
- * certificate (the one that authenticates our authority signing key).
- */
-static int
-check_authority_cert_callback(time_t now, const or_options_t *options)
-{
-  (void)now;
-  (void)options;
-  /* 1e. Periodically, if we're a v3 authority, we check whether our cert is
-   * close to expiring and warn the admin if it is. */
-  v3_authority_check_key_expiry();
-#define CHECK_V3_CERTIFICATE_INTERVAL (5*60)
-  return CHECK_V3_CERTIFICATE_INTERVAL;
-}
-
-/**
- * Scheduled callback: Run directory-authority voting functionality.
- *
- * The schedule is a bit complicated here, so dirvote_act() manages the
- * schedule itself.
- **/
-static int
-dirvote_callback(time_t now, const or_options_t *options)
-{
-  if (!authdir_mode_v3(options)) {
-    tor_assert_nonfatal_unreached();
-    return 3600;
-  }
-
-  time_t next = dirvote_act(options, now);
-  if (BUG(next == TIME_MAX)) {
-    /* This shouldn't be returned unless we called dirvote_act() without
-     * being an authority.  If it happens, maybe our configuration will
-     * fix itself in an hour or so? */
-    return 3600;
-  }
-  return safe_timer_diff(now, next);
-}
-
-/** Reschedule the directory-authority voting event.  Run this whenever the
- * schedule has changed. */
-void
-reschedule_dirvote(const or_options_t *options)
-{
-  if (periodic_events_initialized && authdir_mode_v3(options)) {
-    periodic_event_reschedule(dirvote_event);
-  }
-}
-
 /**
  * Periodic callback: If our consensus is too old, recalculate whether
  * we can actually use it.
@@ -2798,8 +2600,7 @@ dns_servers_relaunch_checks(void)
 {
   if (server_mode(get_options())) {
     dns_reset_correctness_checks();
-    if (periodic_events_initialized) {
-      tor_assert(check_dns_honesty_event);
+    if (check_dns_honesty_event) {
       periodic_event_reschedule(check_dns_honesty_event);
     }
   }
@@ -2809,8 +2610,6 @@ dns_servers_relaunch_checks(void)
 void
 initialize_mainloop_events(void)
 {
-  initialize_periodic_events();
-
   if (!schedule_active_linked_connections_event) {
     schedule_active_linked_connections_event =
       mainloop_event_postloop_new(schedule_active_linked_connections_cb, NULL);
@@ -2828,9 +2627,17 @@ do_main_loop(void)
   /* initialize the periodic events first, so that code that depends on the
    * events being present does not assert.
    */
-  initialize_periodic_events();
+  tor_assert(periodic_events_initialized);
   initialize_mainloop_events();
 
+  periodic_events_connect_all();
+
+  struct timeval one_second = { 1, 0 };
+  initialize_periodic_events_event = tor_evtimer_new(
+                  tor_libevent_get_base(),
+                  initialize_periodic_events_cb, NULL);
+  event_add(initialize_periodic_events_event, &one_second);
+
 #ifdef HAVE_SYSTEMD_209
   uint64_t watchdog_delay;
   /* set up systemd watchdog notification. */

src/core/mainloop/mainloop.h

@@ -62,7 +62,6 @@ void reset_all_main_loop_timers(void);
 void reschedule_descriptor_update_check(void);
 void reschedule_directory_downloads(void);
 void reschedule_or_state_save(void);
-void reschedule_dirvote(const or_options_t *options);
 void mainloop_schedule_postloop_cleanup(void);
 void rescan_periodic_events(const or_options_t *options);
 MOCK_DECL(void, schedule_rescan_periodic_events,(void));
@@ -90,6 +89,7 @@ void mainloop_schedule_shutdown(int delay_sec);
 
 void tor_init_connection_lists(void);
 void initialize_mainloop_events(void);
+void initialize_periodic_events(void);
 void tor_mainloop_free_all(void);
 
 struct token_bucket_rw_t;
@@ -102,7 +102,6 @@ extern struct token_bucket_rw_t global_relayed_bucket;
 #ifdef MAINLOOP_PRIVATE
 STATIC int run_main_loop_until_done(void);
 STATIC void close_closeable_connections(void);
-STATIC void initialize_periodic_events(void);
 STATIC void teardown_periodic_events(void);
 STATIC int get_my_roles(const or_options_t *);
 STATIC int check_network_participation_callback(time_t now,
@@ -113,7 +112,7 @@ extern smartlist_t *connection_array;
 
 /* We need the periodic_event_item_t definition. */
 #include "core/mainloop/periodic.h"
-extern periodic_event_item_t periodic_events[];
+extern periodic_event_item_t mainloop_periodic_events[];
 #endif
 #endif /* defined(MAIN_PRIVATE) */
 

src/core/mainloop/mainloop_sys.c

@@ -0,0 +1,32 @@
+/* Copyright (c) 2001 Matej Pfajfar.
+ * Copyright (c) 2001-2004, Roger Dingledine.
+ * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson.
+ * Copyright (c) 2007-2019, The Tor Project, Inc. */
+/* See LICENSE for licensing information */
+
+#include "core/or/or.h"
+#include "core/mainloop/mainloop_sys.h"
+#include "core/mainloop/mainloop.h"
+
+#include "lib/subsys/subsys.h"
+
+static int
+subsys_mainloop_initialize(void)
+{
+  initialize_periodic_events();
+  return 0;
+}
+
+static void
+subsys_mainloop_shutdown(void)
+{
+  tor_mainloop_free_all();
+}
+
+const struct subsys_fns_t sys_mainloop = {
+  .name = "mainloop",
+  .supported = true,
+  .level = 5,
+  .initialize = subsys_mainloop_initialize,
+  .shutdown = subsys_mainloop_shutdown,
+};

src/core/mainloop/mainloop_sys.h

@@ -0,0 +1,12 @@
+/* Copyright (c) 2001 Matej Pfajfar.
+ * Copyright (c) 2001-2004, Roger Dingledine.
+ * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson.
+ * Copyright (c) 2007-2019, The Tor Project, Inc. */
+/* See LICENSE for licensing information */
+
+#ifndef MAINLOOP_SYS_H
+#define MAINLOOP_SYS_H
+
+extern const struct subsys_fns_t sys_mainloop;
+
+#endif

src/core/mainloop/periodic.c

@@ -6,9 +6,22 @@
  *
  * \brief Generic backend for handling periodic events.
  *
- * The events in this module are used by main.c to track items that need
+ * The events in this module are used to track items that need
  * to fire once every N seconds, possibly picking a new interval each time
- * that they fire.  See periodic_events[] in main.c for examples.
+ * that they fire.  See periodic_events[] in mainloop.c for examples.
+ *
+ * This module manages a global list of periodic_event_item_t objects,
+ * each corresponding to a single event.  To register an event, pass it to
+ * periodic_events_register() when initializing your subsystem.
+ *
+ * Registering an event makes the periodic event subsystem know about it, but
+ * doesn't cause the event to get created immediately.  Before the event can
+ * be started, periodic_event_connect_all() must be called by mainloop.c to
+ * connect all the events to Libevent.
+ *
+ * We expect that periodic_event_item_t objects will be statically allocated;
+ * we set them up and tear them down here, but we don't take ownership of
+ * them.
  */
 
 #include "core/or/or.h"
@@ -24,6 +37,12 @@
  */
 static const int MAX_INTERVAL = 10 * 365 * 86400;
 
+/**
+ * Global list of periodic events that have been registered with
+ * <b>periodic_event_register</a>.
+ **/
+static smartlist_t *the_periodic_events = NULL;
+
 /** Set the event <b>event</b> to run in <b>next_interval</b> seconds from
  * now. */
 static void
@@ -87,15 +106,16 @@ periodic_event_dispatch(mainloop_event_t *ev, void *data)
 void
 periodic_event_reschedule(periodic_event_item_t *event)
 {
-  /* Don't reschedule a disabled event. */
-  if (periodic_event_is_enabled(event)) {
+  /* Don't reschedule a disabled or uninitialized event. */
+  if (event->ev && periodic_event_is_enabled(event)) {
     periodic_event_set_interval(event, 1);
   }
 }
 
-/** Initializes the libevent backend for a periodic event. */
+/** Connects a periodic event to the Libevent backend.  Does not launch the
+ * event immediately. */
 void
-periodic_event_setup(periodic_event_item_t *event)
+periodic_event_connect(periodic_event_item_t *event)
 {
   if (event->ev) { /* Already setup? This is a bug */
     log_err(LD_BUG, "Initial dispatch should only be done once.");
@@ -113,7 +133,7 @@ void
 periodic_event_launch(periodic_event_item_t *event)
 {
   if (! event->ev) { /* Not setup? This is a bug */
-    log_err(LD_BUG, "periodic_event_launch without periodic_event_setup");
+    log_err(LD_BUG, "periodic_event_launch without periodic_event_connect");
     tor_assert(0);
   }
   /* Event already enabled? This is a bug */
@@ -127,9 +147,9 @@ periodic_event_launch(periodic_event_item_t *event)
   periodic_event_dispatch(event->ev, event);
 }
 
-/** Release all storage associated with <b>event</b> */
-void
-periodic_event_destroy(periodic_event_item_t *event)
+/** Disconnect and unregister the periodic event in <b>event</b> */
+static void
+periodic_event_disconnect(periodic_event_item_t *event)
 {
   if (!event)
     return;
@@ -184,3 +204,161 @@ periodic_event_schedule_and_disable(periodic_event_item_t *event)
 
   mainloop_event_activate(event->ev);
 }
+
+/**
+ * Add <b>item</b> to the list of periodic events.
+ *
+ * Note that <b>item</b> should be statically allocated: we do not
+ * take ownership of it.
+ **/
+void
+periodic_events_register(periodic_event_item_t *item)
+{
+  if (!the_periodic_events)
+    the_periodic_events = smartlist_new();
+
+  if (BUG(smartlist_contains(the_periodic_events, item)))
+    return;
+
+  smartlist_add(the_periodic_events, item);
+}
+
+/**
+ * Make all registered periodic events connect to the libevent backend.
+ */
+void
+periodic_events_connect_all(void)
+{
+  if (! the_periodic_events)
+    return;
+
+  SMARTLIST_FOREACH_BEGIN(the_periodic_events, periodic_event_item_t *, item) {
+    if (item->ev)
+      continue;
+    periodic_event_connect(item);
+  } SMARTLIST_FOREACH_END(item);
+}
+
+/**
+ * Reset all the registered periodic events so we'll do all our actions again
+ * as if we just started up.
+ *
+ * Useful if our clock just moved back a long time from the future,
+ * so we don't wait until that future arrives again before acting.
+ */
+void
+periodic_events_reset_all(void)
+{
+  if (! the_periodic_events)
+    return;
+
+  SMARTLIST_FOREACH_BEGIN(the_periodic_events, periodic_event_item_t *, item) {
+    if (!item->ev)
+      continue;
+
+    periodic_event_reschedule(item);
+  } SMARTLIST_FOREACH_END(item);
+}
+
+/**
+ * Return the registered periodic event whose name is <b>name</b>.
+ * Return NULL if no such event is found.
+ */
+periodic_event_item_t *
+periodic_events_find(const char *name)
+{
+  if (! the_periodic_events)
+    return NULL;
+
+  SMARTLIST_FOREACH_BEGIN(the_periodic_events, periodic_event_item_t *, item) {
+    if (strcmp(name, item->name) == 0)
+      return item;
+  } SMARTLIST_FOREACH_END(item);
+  return NULL;
+}
+
+/**
+ * Start or stop registered periodic events, depending on our current set of
+ * roles.
+ *
+ * Invoked when our list of roles, or the net_disabled flag has changed.
+ **/
+void
+periodic_events_rescan_by_roles(int roles, bool net_disabled)
+{
+  if (! the_periodic_events)
+    return;
+
+  SMARTLIST_FOREACH_BEGIN(the_periodic_events, periodic_event_item_t *, item) {
+    if (!item->ev)
+      continue;
+
+    int enable = !!(item->roles & roles);
+
+    /* Handle the event flags. */
+    if (net_disabled &&
+        (item->flags & PERIODIC_EVENT_FLAG_NEED_NET)) {
+      enable = 0;
+    }
+
+    /* Enable the event if needed. It is safe to enable an event that was
+     * already enabled. Same goes for disabling it. */
+    if (enable) {
+      log_debug(LD_GENERAL, "Launching periodic event %s", item->name);
+      periodic_event_enable(item);
+    } else {
+      log_debug(LD_GENERAL, "Disabling periodic event %s", item->name);
+      if (item->flags & PERIODIC_EVENT_FLAG_RUN_ON_DISABLE) {
+        periodic_event_schedule_and_disable(item);
+      } else {
+        periodic_event_disable(item);
+      }
+    }
+  } SMARTLIST_FOREACH_END(item);
+}
+
+/**
+ * Invoked at shutdown: disconnect and unregister all periodic events.
+ *
+ * Does not free the periodic_event_item_t object themselves, because we do
+ * not own them.
+ */
+void
+periodic_events_disconnect_all(void)
+{
+  if (! the_periodic_events)
+    return;
+
+  SMARTLIST_FOREACH_BEGIN(the_periodic_events, periodic_event_item_t *, item) {
+    periodic_event_disconnect(item);
+  } SMARTLIST_FOREACH_END(item);
+
+  smartlist_free(the_periodic_events);
+}
+
+#define LONGEST_TIMER_PERIOD (30 * 86400)
+/** Helper: Return the number of seconds between <b>now</b> and <b>next</b>,
+ * clipped to the range [1 second, LONGEST_TIMER_PERIOD].
+ *
+ * We use this to answer the question, "how many seconds is it from now until
+ * next" in periodic timer callbacks.  Don't use it for other purposes
+ **/
+int
+safe_timer_diff(time_t now, time_t next)
+{
+  if (next > now) {
+    /* There were no computers at signed TIME_MIN (1902 on 32-bit systems),
+     * and nothing that could run Tor. It's a bug if 'next' is around then.
+     * On 64-bit systems with signed TIME_MIN, TIME_MIN is before the Big
+     * Bang. We cannot extrapolate past a singularity, but there was probably
+     * nothing that could run Tor then, either.
+     **/
+    tor_assert(next > TIME_MIN + LONGEST_TIMER_PERIOD);
+
+    if (next - LONGEST_TIMER_PERIOD > now)
+      return LONGEST_TIMER_PERIOD;
+    return (int)(next - now);
+  } else {
+    return 1;
+  }
+}

src/core/mainloop/periodic.h

@@ -83,11 +83,20 @@ periodic_event_is_enabled(const periodic_event_item_t *item)
 }
 
 void periodic_event_launch(periodic_event_item_t *event);
-void periodic_event_setup(periodic_event_item_t *event);
-void periodic_event_destroy(periodic_event_item_t *event);
+void periodic_event_connect(periodic_event_item_t *event);
+//void periodic_event_disconnect(periodic_event_item_t *event);
 void periodic_event_reschedule(periodic_event_item_t *event);
 void periodic_event_enable(periodic_event_item_t *event);
 void periodic_event_disable(periodic_event_item_t *event);
 void periodic_event_schedule_and_disable(periodic_event_item_t *event);
 
+void periodic_events_register(periodic_event_item_t *item);
+void periodic_events_connect_all(void);
+void periodic_events_reset_all(void);
+periodic_event_item_t *periodic_events_find(const char *name);
+void periodic_events_rescan_by_roles(int roles, bool net_disabled);
+void periodic_events_disconnect_all(void);
+
+int safe_timer_diff(time_t now, time_t next);
+
 #endif /* !defined(TOR_PERIODIC_H) */

src/feature/dirauth/dirauth_periodic.c

@@ -0,0 +1,142 @@
+/* Copyright (c) 2001 Matej Pfajfar.
+ * Copyright (c) 2001-2004, Roger Dingledine.
+ * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson.
+ * Copyright (c) 2007-2019, The Tor Project, Inc. */
+/* See LICENSE for licensing information */
+
+#include "core/or/or.h"
+
+#include "app/config/or_options_st.h"
+#include "core/mainloop/netstatus.h"
+#include "feature/dirauth/reachability.h"
+#include "feature/stats/rephist.h"
+
+#include "feature/dirauth/dirvote.h"
+#include "feature/dirauth/dirauth_periodic.h"
+#include "feature/dirauth/authmode.h"
+
+#include "core/mainloop/periodic.h"
+
+#define DECLARE_EVENT(name, roles, flags)         \
+  static periodic_event_item_t name ## _event =   \
+    PERIODIC_EVENT(name,                          \
+                   PERIODIC_EVENT_ROLE_##roles,   \
+                   flags)
+
+#define FL(name) (PERIODIC_EVENT_FLAG_##name)
+
+/**
+ * Periodic callback: if we're an authority, check on our authority
+ * certificate (the one that authenticates our authority signing key).
+ */
+static int
+check_authority_cert_callback(time_t now, const or_options_t *options)
+{
+  (void)now;
+  (void)options;
+  /* 1e. Periodically, if we're a v3 authority, we check whether our cert is
+   * close to expiring and warn the admin if it is. */
+  v3_authority_check_key_expiry();
+#define CHECK_V3_CERTIFICATE_INTERVAL (5*60)
+  return CHECK_V3_CERTIFICATE_INTERVAL;
+}
+
+DECLARE_EVENT(check_authority_cert, DIRAUTH, 0);
+
+/**
+ * Scheduled callback: Run directory-authority voting functionality.
+ *
+ * The schedule is a bit complicated here, so dirvote_act() manages the
+ * schedule itself.
+ **/
+static int
+dirvote_callback(time_t now, const or_options_t *options)
+{
+  if (!authdir_mode_v3(options)) {
+    tor_assert_nonfatal_unreached();
+    return 3600;
+  }
+
+  time_t next = dirvote_act(options, now);
+  if (BUG(next == TIME_MAX)) {
+    /* This shouldn't be returned unless we called dirvote_act() without
+     * being an authority.  If it happens, maybe our configuration will
+     * fix itself in an hour or so? */
+    return 3600;
+  }
+  return safe_timer_diff(now, next);
+}
+
+DECLARE_EVENT(dirvote, DIRAUTH, FL(NEED_NET));
+
+/** Reschedule the directory-authority voting event.  Run this whenever the
+ * schedule has changed. */
+void
+reschedule_dirvote(const or_options_t *options)
+{
+  if (authdir_mode_v3(options)) {
+    periodic_event_reschedule(&dirvote_event);
+  }
+}
+
+/**
+ * Periodic callback: if we're an authority, record our measured stability
+ * information from rephist in an mtbf file.
+ */
+static int
+save_stability_callback(time_t now, const or_options_t *options)
+{
+  if (authdir_mode_tests_reachability(options)) {
+    if (rep_hist_record_mtbf_data(now, 1)<0) {
+      log_warn(LD_GENERAL, "Couldn't store mtbf data.");
+    }
+  }
+#define SAVE_STABILITY_INTERVAL (30*60)
+  return SAVE_STABILITY_INTERVAL;
+}
+
+DECLARE_EVENT(save_stability, AUTHORITIES, 0);
+
+/**
+ * Periodic callback: if we're an authority, make sure we test
+ * the routers on the network for reachability.
+ */
+static int
+launch_reachability_tests_callback(time_t now, const or_options_t *options)
+{
+  if (authdir_mode_tests_reachability(options) &&
+      !net_is_disabled()) {
+    /* try to determine reachability of the other Tor relays */
+    dirserv_test_reachability(now);
+  }
+  return REACHABILITY_TEST_INTERVAL;
+}
+
+DECLARE_EVENT(launch_reachability_tests, AUTHORITIES, FL(NEED_NET));
+
+/**
+ * Periodic callback: if we're an authority, discount the stability
+ * information (and other rephist information) that's older.
+ */
+static int
+downrate_stability_callback(time_t now, const or_options_t *options)
+{
+  (void)options;
+  /* 1d. Periodically, we discount older stability information so that new
+   * stability info counts more, and save the stability information to disk as
+   * appropriate. */
+  time_t next = rep_hist_downrate_old_runs(now);
+  return safe_timer_diff(now, next);
+}
+
+DECLARE_EVENT(downrate_stability, AUTHORITIES, 0);
+
+void
+dirauth_register_periodic_events(void)
+{
+  periodic_events_register(&downrate_stability_event);
+  periodic_events_register(&launch_reachability_tests_event);
+  periodic_events_register(&save_stability_event);
+  periodic_events_register(&check_authority_cert_event);
+  periodic_events_register(&dirvote_event);
+}

src/feature/dirauth/dirauth_periodic.h

@@ -0,0 +1,25 @@
+/* Copyright (c) 2001 Matej Pfajfar.
+ * Copyright (c) 2001-2004, Roger Dingledine.
+ * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson.
+ * Copyright (c) 2007-2019, The Tor Project, Inc. */
+/* See LICENSE for licensing information */
+
+#ifndef DIRVOTE_PERIODIC_H
+#define DIRVOTE_PERIODIC_H
+
+#ifdef HAVE_MODULE_DIRAUTH
+
+void dirauth_register_periodic_events(void);
+void reschedule_dirvote(const or_options_t *options);
+
+#else
+
+static inline void
+reschedule_dirvote(const or_options_t *options)
+{
+  (void)options;
+}
+
+#endif
+
+#endif

src/feature/dirauth/dirauth_sys.c

@@ -0,0 +1,33 @@
+/* Copyright (c) 2001 Matej Pfajfar.
+ * Copyright (c) 2001-2004, Roger Dingledine.
+ * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson.
+ * Copyright (c) 2007-2019, The Tor Project, Inc. */
+/* See LICENSE for licensing information */
+
+#include "core/or/or.h"
+
+#include "feature/dirauth/dirauth_sys.h"
+#include "feature/dirauth/dirvote.h"
+#include "feature/dirauth/dirauth_periodic.h"
+#include "lib/subsys/subsys.h"
+
+static int
+subsys_dirauth_initialize(void)
+{
+  dirauth_register_periodic_events();
+  return 0;
+}
+
+static void
+subsys_dirauth_shutdown(void)
+{
+  dirvote_free_all();
+}
+
+const struct subsys_fns_t sys_dirauth = {
+  .name = "dirauth",
+  .supported = true,
+  .level = 70,
+  .initialize = subsys_dirauth_initialize,
+  .shutdown = subsys_dirauth_shutdown,
+};

src/feature/dirauth/dirauth_sys.h

@@ -0,0 +1,12 @@
+/* Copyright (c) 2001 Matej Pfajfar.
+ * Copyright (c) 2001-2004, Roger Dingledine.
+ * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson.
+ * Copyright (c) 2007-2019, The Tor Project, Inc. */
+/* See LICENSE for licensing information */
+
+#ifndef DIRAUTH_SYS_H
+#define DIRAUTH_SYS_H
+
+extern const struct subsys_fns_t sys_dirauth;
+
+#endif

src/feature/nodelist/networkstatus.c

@@ -82,6 +82,7 @@
 #include "lib/crypt_ops/crypto_rand.h"
 #include "lib/crypt_ops/crypto_util.h"
 
+#include "feature/dirauth/dirauth_periodic.h"
 #include "feature/dirauth/dirvote.h"
 #include "feature/dirauth/authmode.h"
 #include "feature/dirauth/shared_random.h"

src/test/test_periodic_event.c

@@ -51,12 +51,13 @@ test_pe_initialize(void *arg)
    * need to run the main loop and then wait for a second delaying the unit
    * tests. Instead, we'll test the callback work indepedently elsewhere. */
   initialize_periodic_events();
+  periodic_events_connect_all();
   set_network_participation(false);
   rescan_periodic_events(get_options());
 
   /* Validate that all events have been set up. */
-  for (int i = 0; periodic_events[i].name; ++i) {
-    periodic_event_item_t *item = &periodic_events[i];
+  for (int i = 0; mainloop_periodic_events[i].name; ++i) {
+    periodic_event_item_t *item = &mainloop_periodic_events[i];
     tt_assert(item->ev);
     tt_assert(item->fn);
     tt_u64_op(item->last_action_time, OP_EQ, 0);
@@ -89,8 +90,8 @@ test_pe_launch(void *arg)
   /* Hack: We'll set a dumb fn() of each events so they don't get called when
    * dispatching them. We just want to test the state of the callbacks, not
    * the whole code path. */
-  for (int i = 0; periodic_events[i].name; ++i) {
-    periodic_event_item_t *item = &periodic_events[i];
+  for (int i = 0; mainloop_periodic_events[i].name; ++i) {
+    periodic_event_item_t *item = &mainloop_periodic_events[i];
     item->fn = dumb_event_fn;
   }
 
@@ -110,14 +111,15 @@ test_pe_launch(void *arg)
 #endif
 
   initialize_periodic_events();
+  periodic_events_connect_all();
 
   /* Now that we've initialized, rescan the list to launch. */
   periodic_events_on_new_options(options);
 
   int mask = PERIODIC_EVENT_ROLE_CLIENT|PERIODIC_EVENT_ROLE_ALL|
     PERIODIC_EVENT_ROLE_NET_PARTICIPANT;
-  for (int i = 0; periodic_events[i].name; ++i) {
-    periodic_event_item_t *item = &periodic_events[i];
+  for (int i = 0; mainloop_periodic_events[i].name; ++i) {
+    periodic_event_item_t *item = &mainloop_periodic_events[i];
     int should_be_enabled = !!(item->roles & mask);
     tt_int_op(periodic_event_is_enabled(item), OP_EQ, should_be_enabled);
     // enabled or not, the event has not yet been run.
@@ -134,8 +136,8 @@ test_pe_launch(void *arg)
              PERIODIC_EVENT_ROLE_RELAY|PERIODIC_EVENT_ROLE_DIRSERVER|
              PERIODIC_EVENT_ROLE_ALL|PERIODIC_EVENT_ROLE_NET_PARTICIPANT);
 
-  for (int i = 0; periodic_events[i].name; ++i) {
-    periodic_event_item_t *item = &periodic_events[i];
+  for (int i = 0; mainloop_periodic_events[i].name; ++i) {
+    periodic_event_item_t *item = &mainloop_periodic_events[i];
     /* Only Client role should be disabled. */
     if (item->roles == PERIODIC_EVENT_ROLE_CLIENT) {
       tt_int_op(periodic_event_is_enabled(item), OP_EQ, 0);
@@ -156,8 +158,8 @@ test_pe_launch(void *arg)
   set_network_participation(false);
   periodic_events_on_new_options(options);
 
-  for (int i = 0; periodic_events[i].name; ++i) {
-    periodic_event_item_t *item = &periodic_events[i];
+  for (int i = 0; mainloop_periodic_events[i].name; ++i) {
+    periodic_event_item_t *item = &mainloop_periodic_events[i];
     int should_be_enabled = (item->roles & PERIODIC_EVENT_ROLE_ALL) &&
       !(item->flags & PERIODIC_EVENT_FLAG_NEED_NET);
     tt_int_op(periodic_event_is_enabled(item), OP_EQ, should_be_enabled);
@@ -177,8 +179,8 @@ test_pe_launch(void *arg)
    * trigger a rescan of the event disabling the HS service event. */
   to_remove = &service;
 
-  for (int i = 0; periodic_events[i].name; ++i) {
-    periodic_event_item_t *item = &periodic_events[i];
+  for (int i = 0; mainloop_periodic_events[i].name; ++i) {
+    periodic_event_item_t *item = &mainloop_periodic_events[i];
     tt_int_op(periodic_event_is_enabled(item), OP_EQ,
               (item->roles != PERIODIC_EVENT_ROLE_CONTROLEV));
   }
@@ -300,12 +302,13 @@ test_pe_hs_service(void *arg)
   consider_hibernation(time(NULL));
   /* Initialize the events so we can enable them */
   initialize_periodic_events();
+  periodic_events_connect_all();
 
   /* Hack: We'll set a dumb fn() of each events so they don't get called when
    * dispatching them. We just want to test the state of the callbacks, not
    * the whole code path. */
-  for (int i = 0; periodic_events[i].name; ++i) {
-    periodic_event_item_t *item = &periodic_events[i];
+  for (int i = 0; mainloop_periodic_events[i].name; ++i) {
+    periodic_event_item_t *item = &mainloop_periodic_events[i];
     item->fn = dumb_event_fn;
   }
 
@@ -318,8 +321,8 @@ test_pe_hs_service(void *arg)
    * trigger a rescan of the event disabling the HS service event. */
   to_remove = &service;
 
-  for (int i = 0; periodic_events[i].name; ++i) {
-    periodic_event_item_t *item = &periodic_events[i];
+  for (int i = 0; mainloop_periodic_events[i].name; ++i) {
+    periodic_event_item_t *item = &mainloop_periodic_events[i];
     if (item->roles & PERIODIC_EVENT_ROLE_HS_SERVICE) {
       tt_int_op(periodic_event_is_enabled(item), OP_EQ, 1);
     }
@@ -329,8 +332,8 @@ test_pe_hs_service(void *arg)
   /* Remove the service from the global map, it should trigger a rescan and
    * disable the HS service events. */
   remove_service(get_hs_service_map(), &service);
-  for (int i = 0; periodic_events[i].name; ++i) {
-    periodic_event_item_t *item = &periodic_events[i];
+  for (int i = 0; mainloop_periodic_events[i].name; ++i) {
+    periodic_event_item_t *item = &mainloop_periodic_events[i];
     if (item->roles & PERIODIC_EVENT_ROLE_HS_SERVICE) {
       tt_int_op(periodic_event_is_enabled(item), OP_EQ, 0);
     }