Merge remote-tracking branch 'dgoulet/ticket25762_034_05'

src/or/config.c

@@ -903,8 +903,13 @@ set_options(or_options_t *new_val, char **msg)
     smartlist_free(elements);
   }
 
-  if (old_options != global_options)
+  if (old_options != global_options) {
     or_options_free(old_options);
+    /* If we are here it means we've successfully applied the new options and
+     * that the global options have been changed to the new values. We'll
+     * check if we need to remove or add periodic events. */
+    periodic_events_on_new_options(global_options);
+  }
 
   return 0;
 }

src/or/main.c

@@ -150,6 +150,7 @@ static int run_main_loop_until_done(void);
 static void process_signal(int sig);
 static void shutdown_did_not_work_callback(evutil_socket_t fd, short event,
                                            void *arg) ATTR_NORETURN;
+static void rescan_periodic_events(const or_options_t *options);
 
 /********* START VARIABLES **********/
 
@@ -1328,69 +1329,86 @@ static int periodic_events_initialized = 0;
 #undef CALLBACK
 #define CALLBACK(name) \
   static int name ## _callback(time_t, const or_options_t *)
-CALLBACK(rotate_onion_key);
-CALLBACK(check_onion_keys_expiry_time);
-CALLBACK(check_ed_keys);
-CALLBACK(launch_descriptor_fetches);
-CALLBACK(rotate_x509_certificate);
 CALLBACK(add_entropy);
-CALLBACK(launch_reachability_tests);
-CALLBACK(downrate_stability);
-CALLBACK(save_stability);
 CALLBACK(check_authority_cert);
+CALLBACK(check_canonical_channels);
+CALLBACK(check_descriptor);
+CALLBACK(check_dns_honesty);
+CALLBACK(check_ed_keys);
 CALLBACK(check_expired_networkstatus);
-CALLBACK(write_stats_file);
-CALLBACK(record_bridge_stats);
+CALLBACK(check_for_reachability_bw);
+CALLBACK(check_onion_keys_expiry_time);
 CALLBACK(clean_caches);
+CALLBACK(clean_consdiffmgr);
+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(record_bridge_stats);
 CALLBACK(rend_cache_failure_clean);
+CALLBACK(reset_padding_counts);
 CALLBACK(retry_dns);
-CALLBACK(check_descriptor);
-CALLBACK(check_for_reachability_bw);
-CALLBACK(fetch_networkstatus);
 CALLBACK(retry_listeners);
-CALLBACK(expire_old_ciruits_serverside);
-CALLBACK(check_dns_honesty);
+CALLBACK(rotate_onion_key);
+CALLBACK(rotate_x509_certificate);
+CALLBACK(save_stability);
 CALLBACK(write_bridge_ns);
-CALLBACK(heartbeat);
-CALLBACK(clean_consdiffmgr);
-CALLBACK(reset_padding_counts);
-CALLBACK(check_canonical_channels);
-CALLBACK(hs_service);
+CALLBACK(write_stats_file);
 
 #undef CALLBACK
 
 /* Now we declare an array of periodic_event_item_t for each periodic event */
-#define CALLBACK(name) PERIODIC_EVENT(name)
-
-static periodic_event_item_t periodic_events[] = {
-  CALLBACK(rotate_onion_key),
-  CALLBACK(check_onion_keys_expiry_time),
-  CALLBACK(check_ed_keys),
-  CALLBACK(launch_descriptor_fetches),
-  CALLBACK(rotate_x509_certificate),
-  CALLBACK(add_entropy),
-  CALLBACK(launch_reachability_tests),
-  CALLBACK(downrate_stability),
-  CALLBACK(save_stability),
-  CALLBACK(check_authority_cert),
-  CALLBACK(check_expired_networkstatus),
-  CALLBACK(write_stats_file),
-  CALLBACK(record_bridge_stats),
-  CALLBACK(clean_caches),
-  CALLBACK(rend_cache_failure_clean),
-  CALLBACK(retry_dns),
-  CALLBACK(check_descriptor),
-  CALLBACK(check_for_reachability_bw),
-  CALLBACK(fetch_networkstatus),
-  CALLBACK(retry_listeners),
-  CALLBACK(expire_old_ciruits_serverside),
-  CALLBACK(check_dns_honesty),
-  CALLBACK(write_bridge_ns),
-  CALLBACK(heartbeat),
-  CALLBACK(clean_consdiffmgr),
-  CALLBACK(reset_padding_counts),
-  CALLBACK(check_canonical_channels),
-  CALLBACK(hs_service),
+#define CALLBACK(name, r) PERIODIC_EVENT(name, r)
+
+STATIC periodic_event_item_t periodic_events[] = {
+  /* Everyone needs to run those. */
+  CALLBACK(add_entropy, PERIODIC_EVENT_ROLE_ALL),
+  CALLBACK(check_expired_networkstatus, PERIODIC_EVENT_ROLE_ALL),
+  CALLBACK(clean_caches, PERIODIC_EVENT_ROLE_ALL),
+  CALLBACK(fetch_networkstatus, PERIODIC_EVENT_ROLE_ALL),
+  CALLBACK(heartbeat, PERIODIC_EVENT_ROLE_ALL),
+  CALLBACK(launch_descriptor_fetches, PERIODIC_EVENT_ROLE_ALL),
+  CALLBACK(reset_padding_counts, PERIODIC_EVENT_ROLE_ALL),
+  CALLBACK(retry_listeners, PERIODIC_EVENT_ROLE_ALL),
+  CALLBACK(rotate_x509_certificate, PERIODIC_EVENT_ROLE_ALL),
+  CALLBACK(write_stats_file, PERIODIC_EVENT_ROLE_ALL),
+
+  /* Routers (bridge and relay) only. */
+  CALLBACK(check_descriptor, PERIODIC_EVENT_ROLE_ROUTER),
+  CALLBACK(check_ed_keys, PERIODIC_EVENT_ROLE_ROUTER),
+  CALLBACK(check_for_reachability_bw, PERIODIC_EVENT_ROLE_ROUTER),
+  CALLBACK(check_onion_keys_expiry_time, PERIODIC_EVENT_ROLE_ROUTER),
+  CALLBACK(clean_consdiffmgr, PERIODIC_EVENT_ROLE_ROUTER),
+  CALLBACK(expire_old_ciruits_serverside, PERIODIC_EVENT_ROLE_ROUTER),
+  CALLBACK(retry_dns, PERIODIC_EVENT_ROLE_ROUTER),
+  CALLBACK(rotate_onion_key, PERIODIC_EVENT_ROLE_ROUTER),
+
+  /* Authorities (bridge and directory) only. */
+  CALLBACK(downrate_stability, PERIODIC_EVENT_ROLE_AUTHORITIES),
+  CALLBACK(launch_reachability_tests, PERIODIC_EVENT_ROLE_AUTHORITIES),
+  CALLBACK(save_stability, PERIODIC_EVENT_ROLE_AUTHORITIES),
+
+  /* Directory authority only. */
+  CALLBACK(check_authority_cert, PERIODIC_EVENT_ROLE_DIRAUTH),
+
+  /* Relay only. */
+  CALLBACK(check_canonical_channels, PERIODIC_EVENT_ROLE_RELAY),
+  CALLBACK(check_dns_honesty, PERIODIC_EVENT_ROLE_RELAY),
+
+  /* Hidden Service service only. */
+  CALLBACK(hs_service, PERIODIC_EVENT_ROLE_HS_SERVICE),
+
+  /* Bridge only. */
+  CALLBACK(record_bridge_stats, PERIODIC_EVENT_ROLE_BRIDGE),
+
+  /* Client only. */
+  CALLBACK(rend_cache_failure_clean, PERIODIC_EVENT_ROLE_CLIENT),
+
+  /* Bridge Authority only. */
+  CALLBACK(write_bridge_ns, PERIODIC_EVENT_ROLE_BRIDGEAUTH),
   END_OF_PERIODIC_EVENTS
 };
 #undef CALLBACK
@@ -1432,6 +1450,32 @@ find_periodic_event(const char *name)
   return NULL;
 }
 
+/** Return a bitmask of the roles this tor instance is configured for using
+ * the given options. */
+STATIC int
+get_my_roles(const or_options_t *options)
+{
+  tor_assert(options);
+
+  int roles = 0;
+  int is_bridge = options->BridgeRelay;
+  int is_client = any_client_port_set(options);
+  int is_relay = server_mode(options);
+  int is_dirauth = authdir_mode_v3(options);
+  int is_bridgeauth = authdir_mode_bridge(options);
+  int is_hidden_service = !!hs_service_get_num_services() ||
+                          !!rend_num_services();
+
+  if (is_bridge) roles |= PERIODIC_EVENT_ROLE_BRIDGE;
+  if (is_client) roles |= PERIODIC_EVENT_ROLE_CLIENT;
+  if (is_relay) roles |= PERIODIC_EVENT_ROLE_RELAY;
+  if (is_dirauth) roles |= PERIODIC_EVENT_ROLE_DIRAUTH;
+  if (is_bridgeauth) roles |= PERIODIC_EVENT_ROLE_BRIDGEAUTH;
+  if (is_hidden_service) roles |= PERIODIC_EVENT_ROLE_HS_SERVICE;
+
+  return roles;
+}
+
 /** Event to run initialize_periodic_events_cb */
 static struct event *initialize_periodic_events_event = NULL;
 
@@ -1446,11 +1490,10 @@ initialize_periodic_events_cb(evutil_socket_t fd, short events, void *data)
   (void) fd;
   (void) events;
   (void) data;
+
   tor_event_free(initialize_periodic_events_event);
-  int i;
-  for (i = 0; periodic_events[i].name; ++i) {
-    periodic_event_launch(&periodic_events[i]);
-  }
+
+  rescan_periodic_events(get_options());
 }
 
 /** Set up all the members of periodic_events[], and configure them all to be
@@ -1461,6 +1504,7 @@ initialize_periodic_events(void)
   tor_assert(periodic_events_initialized == 0);
   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]);
@@ -1491,6 +1535,44 @@ teardown_periodic_events(void)
   periodic_events_initialized = 0;
 }
 
+/** Do a pass at all our periodic events, disable those we don't need anymore
+ * and enable those we need now using the given options. */
+static void
+rescan_periodic_events(const or_options_t *options)
+{
+  tor_assert(options);
+
+  int roles = get_my_roles(options);
+
+  for (int i = 0; periodic_events[i].name; ++i) {
+    periodic_event_item_t *item = &periodic_events[i];
+
+    /* Enable the event if needed. It is safe to enable an event that was
+     * already enabled. Same goes for disabling it. */
+    if (item->roles & roles) {
+      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);
+      periodic_event_disable(item);
+    }
+  }
+}
+
+/* We just got new options globally set, see if we need to enabled or disable
+ * periodic events. */
+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);
+  }
+}
+
 /**
  * Update our schedule so that we'll check whether we need to update our
  * descriptor immediately, rather than after up to CHECK_DESCRIPTOR_INTERVAL

src/or/main.h

@@ -87,6 +87,8 @@ uint64_t get_main_loop_success_count(void);
 uint64_t get_main_loop_error_count(void);
 uint64_t get_main_loop_idle_count(void);
 
+void periodic_events_on_new_options(const or_options_t *options);
+
 extern time_t time_of_process_start;
 extern int quiet_level;
 extern token_bucket_rw_t global_bucket;
@@ -97,8 +99,13 @@ STATIC void init_connection_lists(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 *options);
 #ifdef TOR_UNIT_TESTS
 extern smartlist_t *connection_array;
+
+/* We need the periodic_event_item_t definition. */
+#include "periodic.h"
+extern periodic_event_item_t periodic_events[];
 #endif
 #endif /* defined(MAIN_PRIVATE) */
 

src/or/networkstatus.c

@@ -1680,7 +1680,7 @@ networkstatus_set_current_consensus_from_ns(networkstatus_t *c,
  * XXXX If we need this elsewhere at any point, we should make it nonstatic
  * XXXX and move it into another file.
  */
-static int
+int
 any_client_port_set(const or_options_t *options)
 {
   return (options->SocksPort_set ||

src/or/networkstatus.h

@@ -140,6 +140,8 @@ void vote_routerstatus_free_(vote_routerstatus_t *rs);
 #define vote_routerstatus_free(rs) \
   FREE_AND_NULL(vote_routerstatus_t, vote_routerstatus_free_, (rs))
 
+int any_client_port_set(const or_options_t *options);
+
 #ifdef NETWORKSTATUS_PRIVATE
 #ifdef TOR_UNIT_TESTS
 STATIC int networkstatus_set_current_consensus_from_ns(networkstatus_t *c,

src/or/periodic.c

@@ -43,12 +43,22 @@ periodic_event_dispatch(mainloop_event_t *ev, void *data)
   periodic_event_item_t *event = data;
   tor_assert(ev == event->ev);
 
+  if (BUG(!periodic_event_is_enabled(event))) {
+    return;
+  }
+
   time_t now = time(NULL);
   const or_options_t *options = get_options();
 //  log_debug(LD_GENERAL, "Dispatching %s", event->name);
   int r = event->fn(now, options);
   int next_interval = 0;
 
+  if (!periodic_event_is_enabled(event)) {
+    /* The event got disabled from inside its callback; no need to
+     * reschedule. */
+    return;
+  }
+
   /* update the last run time if action was taken */
   if (r==0) {
     log_err(LD_BUG, "Invalid return value for periodic event from %s.",
@@ -78,7 +88,10 @@ periodic_event_dispatch(mainloop_event_t *ev, void *data)
 void
 periodic_event_reschedule(periodic_event_item_t *event)
 {
-  periodic_event_set_interval(event, 1);
+  /* Don't reschedule a disabled event. */
+  if (periodic_event_is_enabled(event)) {
+    periodic_event_set_interval(event, 1);
+  }
 }
 
 /** Initializes the libevent backend for a periodic event. */
@@ -104,8 +117,14 @@ periodic_event_launch(periodic_event_item_t *event)
     log_err(LD_BUG, "periodic_event_launch without periodic_event_setup");
     tor_assert(0);
   }
+  /* Event already enabled? This is a bug */
+  if (periodic_event_is_enabled(event)) {
+    log_err(LD_BUG, "periodic_event_launch on an already enabled event");
+    tor_assert(0);
+  }
 
   // Initial dispatch
+  event->enabled = 1;
   periodic_event_dispatch(event->ev, event);
 }
 
@@ -119,3 +138,33 @@ periodic_event_destroy(periodic_event_item_t *event)
   event->last_action_time = 0;
 }
 
+/** Enable the given event which means the event is launched and then the
+ * event's enabled flag is set. This can be called for an event that is
+ * already enabled. */
+void
+periodic_event_enable(periodic_event_item_t *event)
+{
+  tor_assert(event);
+  /* Safely and silently ignore if this event is already enabled. */
+  if (periodic_event_is_enabled(event)) {
+    return;
+  }
+
+  periodic_event_launch(event);
+}
+
+/** Disable the given event which means the event is destroyed and then the
+ * event's enabled flag is unset. This can be called for an event that is
+ * already disabled. */
+void
+periodic_event_disable(periodic_event_item_t *event)
+{
+  tor_assert(event);
+  /* Safely and silently ignore if this event is already disabled. */
+  if (!periodic_event_is_enabled(event)) {
+    return;
+  }
+  mainloop_event_cancel(event->ev);
+  event->enabled = 0;
+}
+

src/or/periodic.h

@@ -6,6 +6,29 @@
 
 #define PERIODIC_EVENT_NO_UPDATE (-1)
 
+/* Tor roles for which a periodic event item is for. An event can be for
+ * multiple roles, they can be combined. */
+#define PERIODIC_EVENT_ROLE_CLIENT      (1U << 0)
+#define PERIODIC_EVENT_ROLE_RELAY       (1U << 1)
+#define PERIODIC_EVENT_ROLE_BRIDGE      (1U << 2)
+#define PERIODIC_EVENT_ROLE_DIRAUTH     (1U << 3)
+#define PERIODIC_EVENT_ROLE_BRIDGEAUTH  (1U << 4)
+#define PERIODIC_EVENT_ROLE_HS_SERVICE  (1U << 5)
+
+/* Helper macro to make it a bit less annoying to defined groups of roles that
+ * are often used. */
+
+/* Router that is a Bridge or Relay. */
+#define PERIODIC_EVENT_ROLE_ROUTER \
+  (PERIODIC_EVENT_ROLE_BRIDGE | PERIODIC_EVENT_ROLE_RELAY)
+/* Authorities that is both bridge and directory. */
+#define PERIODIC_EVENT_ROLE_AUTHORITIES \
+  (PERIODIC_EVENT_ROLE_BRIDGEAUTH | PERIODIC_EVENT_ROLE_DIRAUTH)
+/* All roles. */
+#define PERIODIC_EVENT_ROLE_ALL \
+  (PERIODIC_EVENT_ROLE_AUTHORITIES | PERIODIC_EVENT_ROLE_CLIENT | \
+   PERIODIC_EVENT_ROLE_HS_SERVICE | PERIODIC_EVENT_ROLE_ROUTER)
+
 /** Callback function for a periodic event to take action.  The return value
 * influences the next time the function will get called.  Return
 * PERIODIC_EVENT_NO_UPDATE to not update <b>last_action_time</b> and be polled
@@ -23,16 +46,31 @@ typedef struct periodic_event_item_t {
   struct mainloop_event_t *ev; /**< Libevent callback we're using to implement
                                 * this */
   const char *name; /**< Name of the function -- for debug */
+
+  /* Bitmask of roles define above for which this event applies. */
+  uint32_t roles;
+  /* Indicate that this event has been enabled that is scheduled. */
+  unsigned int enabled : 1;
 } periodic_event_item_t;
 
 /** events will get their interval from first execution */
-#define PERIODIC_EVENT(fn) { fn##_callback, 0, NULL, #fn }
-#define END_OF_PERIODIC_EVENTS { NULL, 0, NULL, NULL }
+#define PERIODIC_EVENT(fn, r) { fn##_callback, 0, NULL, #fn, r, 0 }
+#define END_OF_PERIODIC_EVENTS { NULL, 0, NULL, NULL, 0, 0 }
+
+/* Return true iff the given event was setup before thus is enabled to be
+ * scheduled. */
+static inline int
+periodic_event_is_enabled(const periodic_event_item_t *item)
+{
+  return item->enabled;
+}
 
 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_reschedule(periodic_event_item_t *event);
+void periodic_event_enable(periodic_event_item_t *event);
+void periodic_event_disable(periodic_event_item_t *event);
 
 #endif /* !defined(TOR_PERIODIC_H) */
 

src/test/include.am

@@ -144,6 +144,7 @@ src_test_test_SOURCES = \
 	src/test/test_oom.c \
 	src/test/test_oos.c \
 	src/test/test_options.c \
+	src/test/test_periodic_event.c \
 	src/test/test_policy.c \
 	src/test/test_procmon.c \
 	src/test/test_proto_http.c \

src/test/test.c

@@ -863,6 +863,7 @@ struct testgroup_t testgroups[] = {
   { "oom/", oom_tests },
   { "oos/", oos_tests },
   { "options/", options_tests },
+  { "periodic-event/" , periodic_event_tests },
   { "policy/" , policy_tests },
   { "procmon/", procmon_tests },
   { "proto/http/", proto_http_tests },

src/test/test.h

@@ -239,6 +239,7 @@ extern struct testcase_t nodelist_tests[];
 extern struct testcase_t oom_tests[];
 extern struct testcase_t oos_tests[];
 extern struct testcase_t options_tests[];
+extern struct testcase_t periodic_event_tests[];
 extern struct testcase_t policy_tests[];
 extern struct testcase_t procmon_tests[];
 extern struct testcase_t proto_http_tests[];

src/test/test_periodic_event.c

@@ -0,0 +1,256 @@
+/* Copyright (c) 2018, The Tor Project, Inc. */
+/* See LICENSE for licensing information */
+
+/**
+ * \file test_periodic_event.c
+ * \brief Test the periodic events that Tor uses for different roles. They are
+ *        part of the libevent mainloop
+ */
+
+#define CONFIG_PRIVATE
+#define HS_SERVICE_PRIVATE
+#define MAIN_PRIVATE
+
+#include "test.h"
+#include "test_helpers.h"
+
+#include "or.h"
+#include "config.h"
+#include "hs_service.h"
+#include "main.h"
+#include "periodic.h"
+
+/** Helper function: This is replaced in some tests for the event callbacks so
+ * we don't actually go into the code path of those callbacks. */
+static int
+dumb_event_fn(time_t now, const or_options_t *options)
+{
+  (void) now;
+  (void) options;
+
+  /* Will get rescheduled in 300 seconds. It just can't be 0. */
+  return 300;
+}
+
+static void
+register_dummy_hidden_service(hs_service_t *service)
+{
+  memset(service, 0, sizeof(hs_service_t));
+  memset(&service->keys.identity_pk, 'A', sizeof(service->keys.identity_pk));
+  (void) register_service(get_hs_service_map(), service);
+}
+
+static void
+test_pe_initialize(void *arg)
+{
+  (void) arg;
+
+  /* Initialize the events but the callback won't get called since we would
+   * 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();
+
+  /* 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];
+    tt_assert(item->ev);
+    tt_assert(item->fn);
+    tt_u64_op(item->last_action_time, OP_EQ, 0);
+    /* Every event must have role(s) assign to it. This is done statically. */
+    tt_u64_op(item->roles, OP_NE, 0);
+    tt_uint_op(periodic_event_is_enabled(item), OP_EQ, 0);
+  }
+
+ done:
+  teardown_periodic_events();
+}
+
+static void
+test_pe_launch(void *arg)
+{
+  hs_service_t service;
+  or_options_t *options;
+
+  (void) arg;
+
+  hs_init();
+
+  /* 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];
+    item->fn = dumb_event_fn;
+  }
+
+  /* Lets make sure that before intialization, we can't scan the periodic
+   * events list and launch them. Lets try by being a Client. */
+  options = get_options_mutable();
+  options->SocksPort_set = 1;
+  periodic_events_on_new_options(options);
+  for (int i = 0; periodic_events[i].name; ++i) {
+    periodic_event_item_t *item = &periodic_events[i];
+    tt_int_op(periodic_event_is_enabled(item), OP_EQ, 0);
+  }
+
+  initialize_periodic_events();
+
+  /* Now that we've initialized, rescan the list to launch. */
+  periodic_events_on_new_options(options);
+
+  for (int i = 0; periodic_events[i].name; ++i) {
+    periodic_event_item_t *item = &periodic_events[i];
+    if (item->roles & PERIODIC_EVENT_ROLE_CLIENT) {
+      tt_int_op(periodic_event_is_enabled(item), OP_EQ, 1);
+      tt_u64_op(item->last_action_time, OP_NE, 0);
+    } else {
+      tt_int_op(periodic_event_is_enabled(item), OP_EQ, 0);
+      tt_u64_op(item->last_action_time, OP_EQ, 0);
+    }
+  }
+
+  /* Remove Client but become a Relay. */
+  options->SocksPort_set = 0;
+  options->ORPort_set = 1;
+  periodic_events_on_new_options(options);
+
+  for (int i = 0; periodic_events[i].name; ++i) {
+    periodic_event_item_t *item = &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);
+      /* Was previously enabled so they should never be to 0. */
+      tt_u64_op(item->last_action_time, OP_NE, 0);
+    }
+    if (item->roles & PERIODIC_EVENT_ROLE_RELAY) {
+      tt_int_op(periodic_event_is_enabled(item), OP_EQ, 1);
+      tt_u64_op(item->last_action_time, OP_NE, 0);
+    }
+    /* Non Relay role should be disabled! */
+    if (!(item->roles & PERIODIC_EVENT_ROLE_RELAY)) {
+      tt_int_op(periodic_event_is_enabled(item), OP_EQ, 0);
+    }
+  }
+
+  /* Disable everything and we'll enable them ALL. */
+  options->SocksPort_set = 0;
+  options->ORPort_set = 0;
+  periodic_events_on_new_options(options);
+
+  for (int i = 0; periodic_events[i].name; ++i) {
+    periodic_event_item_t *item = &periodic_events[i];
+    tt_int_op(periodic_event_is_enabled(item), OP_EQ, 0);
+  }
+
+  /* Enable everything. */
+  options->SocksPort_set = 1; options->ORPort_set = 1;
+  options->BridgeRelay = 1; options->AuthoritativeDir = 1;
+  options->V3AuthoritativeDir = 1; options->BridgeAuthoritativeDir = 1;
+  register_dummy_hidden_service(&service);
+  periodic_events_on_new_options(options);
+  /* Remove it now so the hs_free_all() doesn't try to free stack memory. */
+  remove_service(get_hs_service_map(), &service);
+
+  for (int i = 0; periodic_events[i].name; ++i) {
+    periodic_event_item_t *item = &periodic_events[i];
+    tt_int_op(periodic_event_is_enabled(item), OP_EQ, 1);
+  }
+
+ done:
+  hs_free_all();
+}
+
+static void
+test_pe_get_roles(void *arg)
+{
+  int roles;
+
+  (void) arg;
+
+  /* Just so the HS global map exists. */
+  hs_init();
+
+  or_options_t *options = get_options_mutable();
+  tt_assert(options);
+
+  /* Nothing configured, should be no roles. */
+  roles = get_my_roles(options);
+  tt_int_op(roles, OP_EQ, 0);
+
+  /* Indicate we have a SocksPort, roles should be come Client. */
+  options->SocksPort_set = 1;
+  roles = get_my_roles(options);
+  tt_int_op(roles, OP_EQ, PERIODIC_EVENT_ROLE_CLIENT);
+
+  /* Now, we'll add a ORPort so should now be a Relay + Client. */
+  options->ORPort_set = 1;
+  roles = get_my_roles(options);
+  tt_int_op(roles, OP_EQ,
+            (PERIODIC_EVENT_ROLE_CLIENT | PERIODIC_EVENT_ROLE_RELAY));
+
+  /* Now add a Bridge. */
+  options->BridgeRelay = 1;
+  roles = get_my_roles(options);
+  tt_int_op(roles, OP_EQ,
+            (PERIODIC_EVENT_ROLE_CLIENT | PERIODIC_EVENT_ROLE_RELAY |
+             PERIODIC_EVENT_ROLE_BRIDGE));
+  tt_assert(roles & PERIODIC_EVENT_ROLE_ROUTER);
+  /* Unset client so we can solely test Router role. */
+  options->SocksPort_set = 0;
+  roles = get_my_roles(options);
+  tt_int_op(roles, OP_EQ, PERIODIC_EVENT_ROLE_ROUTER);
+
+  /* Reset options so we can test authorities. */
+  options->SocksPort_set = 0;
+  options->ORPort_set = 0;
+  options->BridgeRelay = 0;
+  roles = get_my_roles(options);
+  tt_int_op(roles, OP_EQ, 0);
+
+  /* Now upgrade to Dirauth. */
+  options->AuthoritativeDir = 1;
+  options->V3AuthoritativeDir = 1;
+  roles = get_my_roles(options);
+  tt_int_op(roles, OP_EQ, PERIODIC_EVENT_ROLE_DIRAUTH);
+  tt_assert(roles & PERIODIC_EVENT_ROLE_AUTHORITIES);
+
+  /* Now Bridge Authority. */
+  options->V3AuthoritativeDir = 0;
+  options->BridgeAuthoritativeDir = 1;
+  roles = get_my_roles(options);
+  tt_int_op(roles, OP_EQ, PERIODIC_EVENT_ROLE_BRIDGEAUTH);
+  tt_assert(roles & PERIODIC_EVENT_ROLE_AUTHORITIES);
+
+  /* Move that bridge auth to become a relay. */
+  options->ORPort_set = 1;
+  roles = get_my_roles(options);
+  tt_int_op(roles, OP_EQ,
+            (PERIODIC_EVENT_ROLE_BRIDGEAUTH | PERIODIC_EVENT_ROLE_RELAY));
+  tt_assert(roles & PERIODIC_EVENT_ROLE_AUTHORITIES);
+
+  /* And now an Hidden service. */
+  hs_service_t service;
+  register_dummy_hidden_service(&service);
+  roles = get_my_roles(options);
+  /* Remove it now so the hs_free_all() doesn't try to free stack memory. */
+  remove_service(get_hs_service_map(), &service);
+  tt_int_op(roles, OP_EQ,
+            (PERIODIC_EVENT_ROLE_BRIDGEAUTH | PERIODIC_EVENT_ROLE_RELAY |
+             PERIODIC_EVENT_ROLE_HS_SERVICE));
+  tt_assert(roles & PERIODIC_EVENT_ROLE_AUTHORITIES);
+
+ done:
+  hs_free_all();
+}
+
+#define PE_TEST(name) \
+  { #name, test_pe_## name , TT_FORK, NULL, NULL }
+
+struct testcase_t periodic_event_tests[] = {
+  PE_TEST(initialize),
+  PE_TEST(launch),
+  PE_TEST(get_roles),
+
+  END_OF_TESTCASES
+};
+