commit 233835e14f2ea1878804ba3a81e6f24e3419eafc Author: Nick Mathewson nickm@torproject.org Date: Thu Apr 25 10:09:36 2019 -0400

Move the responsibility for listing periodic events to periodic.c

The end goal here is to move the periodic callback to their respective modules, so that mainloop.c doesn't have to include so many other things.

This patch doesn't actually move any of the callbacks out of mainloop.c yet. --- src/core/mainloop/mainloop.c | 69 ++++----------------- src/core/mainloop/mainloop.h | 2 +- src/core/mainloop/periodic.c | 137 ++++++++++++++++++++++++++++++++++++++++- src/core/mainloop/periodic.h | 7 +++ src/test/test_periodic_event.c | 36 +++++------ 5 files changed, 173 insertions(+), 78 deletions(-)

diff --git a/src/core/mainloop/mainloop.c b/src/core/mainloop/mainloop.c index c9f2b0d89..b32532c76 100644 --- a/src/core/mainloop/mainloop.c +++ b/src/core/mainloop/mainloop.c @@ -1387,7 +1387,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. */ @@ -1485,24 +1485,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,8 +1548,8 @@ 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. */ +/** Set up all the members of mainloop_periodic_events[], and configure them + * all to be launched from a callback. */ STATIC void initialize_periodic_events(void) { @@ -1575,14 +1558,15 @@ 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_add(&mainloop_periodic_events[i]); }

+ /* Set up all periodic events. We'll launch them by roles. */ + periodic_events_setup_all(); + #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); @@ -1602,10 +1586,7 @@ initialize_periodic_events(void) STATIC void teardown_periodic_events(void) { - int i; - for (i = 0; periodic_events[i].name; ++i) { - periodic_event_destroy(&periodic_events[i]); - } + periodic_events_destroy_all(); periodic_events_initialized = 0; }

@@ -1647,33 +1628,7 @@ rescan_periodic_events(const or_options_t *options) 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 diff --git a/src/core/mainloop/mainloop.h b/src/core/mainloop/mainloop.h index 6ed93fa90..850918c35 100644 --- a/src/core/mainloop/mainloop.h +++ b/src/core/mainloop/mainloop.h @@ -113,7 +113,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) */

diff --git a/src/core/mainloop/periodic.c b/src/core/mainloop/periodic.c index c0363b15e..706dbc1b5 100644 --- a/src/core/mainloop/periodic.c +++ b/src/core/mainloop/periodic.c @@ -6,9 +6,17 @@ * * \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_add() when initializing your subsystem. + * + * 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 +32,11 @@ */ static const int MAX_INTERVAL = 10 * 365 * 86400;

+/** + * + **/ +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 @@ -184,3 +197,123 @@ 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_add(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); +} + +/** Set up all not-previously setup periodic events. */ +void +periodic_events_setup_all(void) +{ + if (! the_periodic_events) + return; + + SMARTLIST_FOREACH_BEGIN(the_periodic_events, periodic_event_item_t *, item) { + if (item->ev) + continue; + periodic_event_setup(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) { + 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) { + 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: free resources used in this module. + * + * Does not free the periodic_event_item_t object themselves, because we do + * not own them. */ +void +periodic_events_destroy_all(void) +{ + if (! the_periodic_events) + return; + + SMARTLIST_FOREACH_BEGIN(the_periodic_events, periodic_event_item_t *, item) { + periodic_event_destroy(item); + } SMARTLIST_FOREACH_END(item); + + smartlist_free(the_periodic_events); +} diff --git a/src/core/mainloop/periodic.h b/src/core/mainloop/periodic.h index 344fc9ad2..a021a141d 100644 --- a/src/core/mainloop/periodic.h +++ b/src/core/mainloop/periodic.h @@ -90,4 +90,11 @@ 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_add(periodic_event_item_t *item); +void periodic_events_setup_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_destroy_all(void); + #endif /* !defined(TOR_PERIODIC_H) */ diff --git a/src/test/test_periodic_event.c b/src/test/test_periodic_event.c index ebac20838..645274d37 100644 --- a/src/test/test_periodic_event.c +++ b/src/test/test_periodic_event.c @@ -55,8 +55,8 @@ test_pe_initialize(void *arg) 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 +89,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; }

@@ -116,8 +116,8 @@ test_pe_launch(void *arg)

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 +134,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 +156,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 +177,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)); } @@ -304,8 +304,8 @@ test_pe_hs_service(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; }

@@ -318,8 +318,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 +329,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); }