[tor-commits] [tor/master] Add an API for a scheduled/manually activated event in the mainloop

commit 871ff0006d052967534fb6ce31465b55021888f0 Author: Nick Mathewson nickm@torproject.org Date: Tue Oct 3 10:44:45 2017 -0400

Add an API for a scheduled/manually activated event in the mainloop

Using this API lets us remove event2/event.h usage from half a dozen modules, to better isolate libevent. Implements part of ticket 23750. --- src/common/compat_libevent.c | 115 +++++++++++++++++++++++++++++++++++++++++++ src/common/compat_libevent.h | 11 +++++ src/common/timers.c | 18 +++---- src/or/control.c | 20 +++----- src/or/main.c | 11 ++--- src/or/periodic.c | 21 +++----- src/or/periodic.h | 5 +- src/or/scheduler.c | 25 ++++------ src/or/scheduler.h | 4 +- src/or/scheduler_kist.c | 4 +- src/or/scheduler_vanilla.c | 4 +- 11 files changed, 170 insertions(+), 68 deletions(-)

diff --git a/src/common/compat_libevent.c b/src/common/compat_libevent.c index 735385557..f453a8e87 100644 --- a/src/common/compat_libevent.c +++ b/src/common/compat_libevent.c @@ -221,6 +221,121 @@ periodic_timer_free_(periodic_timer_t *timer) tor_free(timer); }

+/** + * Type used to represent events that run directly from the main loop, + * either because they are activated from elsewhere in the code, or + * because they have a simple timeout. + * + * We use this type to avoid exposing Libevent's API throughout the rest + * of the codebase. + * + * This type can't be used for all events: it doesn't handle events that + * are triggered by signals or by sockets. + */ +struct mainloop_event_t { + struct event *ev; + void (*cb)(mainloop_event_t *, void *); + void *userdata; +}; + +/** + * Internal: Implements mainloop event using a libevent event. + */ +static void +mainloop_event_cb(evutil_socket_t fd, short what, void *arg) +{ + (void)fd; + (void)what; + mainloop_event_t *mev = arg; + mev->cb(mev, mev->userdata); +} + +/** + * Create and return a new mainloop_event_t to run the function <b>cb</b>. + * + * When run, the callback function will be passed the mainloop_event_t + * and <b>userdata</b> as its arguments. The <b>userdata</b> pointer + * must remain valid for as long as the mainloop_event_t event exists: + * it is your responsibility to free it. + * + * The event is not scheduled by default: Use mainloop_event_activate() + * or mainloop_event_schedule() to make it run. + */ +mainloop_event_t * +mainloop_event_new(void (*cb)(mainloop_event_t *, void *), + void *userdata) +{ + tor_assert(cb); + + struct event_base *base = tor_libevent_get_base(); + mainloop_event_t *mev = tor_malloc_zero(sizeof(mainloop_event_t)); + mev->ev = tor_event_new(base, -1, 0, mainloop_event_cb, mev); + tor_assert(mev->ev); + mev->cb = cb; + mev->userdata = userdata; + return mev; +} + +/** + * Schedule <b>event</b> to run in the main loop, immediately. If it is + * not scheduled, it will run anyway. If it is already scheduled to run + * later, it will run now instead. This function will have no effect if + * the event is already scheduled to run. + * + * This function may only be called from the main thread. + */ +void +mainloop_event_activate(mainloop_event_t *event) +{ + tor_assert(event); + event_active(event->ev, EV_READ, 1); +} + +/** Schedule <b>event</b> to run in the main loop, after a delay of <b>tv</b>. + * + * If the event is scheduled for a different time, cancel it and run + * after this delay instead. If the event is currently pending to run + * <em>now</b>, has no effect. + * + * Do not call this function with <b>tv</b> == NULL -- use + * mainloop_event_activate() instead. + * + * This function may only be called from the main thread. + */ +int +mainloop_event_schedule(mainloop_event_t *event, const struct timeval *tv) +{ + tor_assert(event); + if (BUG(tv == NULL)) { + // LCOV_EXCL_START + mainloop_event_activate(event); + return 0; + // LCOV_EXCL_STOP + } + return event_add(event->ev, tv); +} + +/** Cancel <b>event</b> if it is currently active or pending. (Do nothing if + * the event is not currently active or pending.) */ +void +mainloop_event_cancel(mainloop_event_t *event) +{ + if (!event) + return; + event_del(event->ev); +} + +/** Cancel <b>event</b> and release all storage associated with it. */ +void +mainloop_event_free_(mainloop_event_t *event) +{ + if (!event) + return; + tor_event_free(event->ev); + memset(event, 0xb8, sizeof(*event)); + tor_free(event); +} + int tor_init_libevent_rng(void) { diff --git a/src/common/compat_libevent.h b/src/common/compat_libevent.h index 318697864..711c3a2eb 100644 --- a/src/common/compat_libevent.h +++ b/src/common/compat_libevent.h @@ -34,6 +34,17 @@ void periodic_timer_free_(periodic_timer_t *); #define periodic_timer_free(t) \ FREE_AND_NULL(periodic_timer_t, periodic_timer_free_, (t))

+typedef struct mainloop_event_t mainloop_event_t; +mainloop_event_t *mainloop_event_new(void (*cb)(mainloop_event_t *, void *), + void *userdata); +void mainloop_event_activate(mainloop_event_t *event); +int mainloop_event_schedule(mainloop_event_t *event, + const struct timeval *delay); +void mainloop_event_cancel(mainloop_event_t *event); +void mainloop_event_free_(mainloop_event_t *event); +#define mainloop_event_free(event) \ + FREE_AND_NULL(mainloop_event_t, mainloop_event_free_, (event)) + #define tor_event_base_loopexit event_base_loopexit #define tor_event_base_loopbreak event_base_loopbreak

diff --git a/src/common/timers.c b/src/common/timers.c index 552080b11..a90817da1 100644 --- a/src/common/timers.c +++ b/src/common/timers.c @@ -37,8 +37,6 @@ #include "torlog.h" #include "util.h"

-#include <event2/event.h> - struct timeout_cb { timer_cb_fn_t cb; void *arg; @@ -69,7 +67,7 @@ struct timeout_cb { #include "src/ext/timeouts/timeout.c"

static struct timeouts *global_timeouts = NULL; -static struct event *global_timer_event = NULL; +static struct mainloop_event_t *global_timer_event = NULL;

static monotime_t start_of_time;

@@ -147,7 +145,7 @@ libevent_timer_reschedule(void) if (delay > MIN_CHECK_TICKS) delay = MIN_CHECK_TICKS; timeout_to_tv(delay, &d); - event_add(global_timer_event, &d); + mainloop_event_schedule(global_timer_event, &d); }

/** Run the callback of every timer that has expired, based on the current @@ -170,10 +168,9 @@ timers_run_pending(void) * have fired, activate their callbacks, and reschedule the libevent timer. */ static void -libevent_timer_callback(evutil_socket_t fd, short what, void *arg) +libevent_timer_callback(mainloop_event_t *ev, void *arg) { - (void)fd; - (void)what; + (void)ev; (void)arg;

timers_run_pending(); @@ -203,9 +200,8 @@ timers_initialize(void) monotime_init(); monotime_get(&start_of_time);

- struct event *timer_event; - timer_event = tor_event_new(tor_libevent_get_base(), - -1, 0, libevent_timer_callback, NULL); + mainloop_event_t *timer_event; + timer_event = mainloop_event_new(libevent_timer_callback, NULL); tor_assert(timer_event); global_timer_event = timer_event;

@@ -219,7 +215,7 @@ void timers_shutdown(void) { if (global_timer_event) { - tor_event_free(global_timer_event); + mainloop_event_free(global_timer_event); global_timer_event = NULL; } if (global_timeouts) { diff --git a/src/or/control.c b/src/or/control.c index 5cac0e172..5a2fae64e 100644 --- a/src/or/control.c +++ b/src/or/control.c @@ -83,8 +83,6 @@ #include <sys/resource.h> #endif

-#include <event2/event.h> - #include "crypto_s2k.h" #include "procmon.h"

@@ -216,7 +214,7 @@ static void orconn_target_get_name(char *buf, size_t len, static int get_cached_network_liveness(void); static void set_cached_network_liveness(int liveness);

-static void flush_queued_events_cb(evutil_socket_t fd, short what, void *arg); +static void flush_queued_events_cb(mainloop_event_t *event, void *arg);

static char * download_status_to_string(const download_status_t *dl);

@@ -691,7 +689,7 @@ static tor_mutex_t *queued_control_events_lock = NULL;

/** An event that should fire in order to flush the contents of * queued_control_events. */ -static struct event *flush_queued_events_event = NULL; +static mainloop_event_t *flush_queued_events_event = NULL;

void control_initialize_event_queue(void) @@ -703,9 +701,8 @@ control_initialize_event_queue(void) if (flush_queued_events_event == NULL) { struct event_base *b = tor_libevent_get_base(); if (b) { - flush_queued_events_event = tor_event_new(b, - -1, 0, flush_queued_events_cb, - NULL); + flush_queued_events_event = + mainloop_event_new(flush_queued_events_cb, NULL); tor_assert(flush_queued_events_event); } } @@ -781,7 +778,7 @@ queue_control_event_string,(uint16_t event, char *msg)) */ if (activate_event) { tor_assert(flush_queued_events_event); - event_active(flush_queued_events_event, EV_READ, 1); + mainloop_event_activate(flush_queued_events_event); } }

@@ -863,10 +860,9 @@ queued_events_flush_all(int force) /** Libevent callback: Flushes pending events to controllers that are * interested in them. */ static void -flush_queued_events_cb(evutil_socket_t fd, short what, void *arg) +flush_queued_events_cb(mainloop_event_t *event, void *arg) { - (void) fd; - (void) what; + (void) event; (void) arg; queued_events_flush_all(0); } @@ -7608,7 +7604,7 @@ control_free_all(void) smartlist_free(queued_events); } if (flush_queued_events_event) { - tor_event_free(flush_queued_events_event); + mainloop_event_free(flush_queued_events_event); flush_queued_events_event = NULL; } bootstrap_percent = BOOTSTRAP_STATUS_UNDEF; diff --git a/src/or/main.c b/src/or/main.c index a0d2ae075..9f26732a2 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -1060,9 +1060,8 @@ conn_close_if_marked(int i) * reason. */ static void -directory_all_unreachable_cb(evutil_socket_t fd, short event, void *arg) +directory_all_unreachable_cb(mainloop_event_t *event, void *arg) { - (void)fd; (void)event; (void)arg;

@@ -1082,7 +1081,7 @@ directory_all_unreachable_cb(evutil_socket_t fd, short event, void *arg) control_event_general_error("DIR_ALL_UNREACHABLE"); }

-static struct event *directory_all_unreachable_cb_event = NULL; +static mainloop_event_t *directory_all_unreachable_cb_event = NULL;

/** We've just tried every dirserver we know about, and none of * them were reachable. Assume the network is down. Change state @@ -1099,12 +1098,11 @@ directory_all_unreachable(time_t now)

if (!directory_all_unreachable_cb_event) { directory_all_unreachable_cb_event = - tor_event_new(tor_libevent_get_base(), - -1, EV_READ, directory_all_unreachable_cb, NULL); + mainloop_event_new(directory_all_unreachable_cb, NULL); tor_assert(directory_all_unreachable_cb_event); }

- event_active(directory_all_unreachable_cb_event, EV_READ, 1); + mainloop_event_activate(directory_all_unreachable_cb_event); }

/** This function is called whenever we successfully pull down some new @@ -3525,6 +3523,7 @@ tor_free_all(int postfork) periodic_timer_free(refill_timer); tor_event_free(shutdown_did_not_work_event); tor_event_free(initialize_periodic_events_event); + mainloop_event_free(directory_all_unreachable_cb_event);

#ifdef HAVE_SYSTEMD_209 periodic_timer_free(systemd_watchdog_timer); diff --git a/src/or/periodic.c b/src/or/periodic.c index 6896b41c8..fa40965de 100644 --- a/src/or/periodic.c +++ b/src/or/periodic.c @@ -16,8 +16,6 @@ #include "config.h" #include "periodic.h"

-#include <event2/event.h> - /** We disable any interval greater than this number of seconds, on the * grounds that it is probably an absolute time mistakenly passed in as a * relative time. @@ -34,17 +32,16 @@ periodic_event_set_interval(periodic_event_item_t *event, struct timeval tv; tv.tv_sec = next_interval; tv.tv_usec = 0; - event_add(event->ev, &tv); + mainloop_event_schedule(event->ev, &tv); }

/** Wraps dispatches for periodic events, <b>data</b> will be a pointer to the * event that needs to be called */ static void -periodic_event_dispatch(evutil_socket_t fd, short what, void *data) +periodic_event_dispatch(mainloop_event_t *ev, void *data) { - (void)fd; - (void)what; periodic_event_item_t *event = data; + tor_assert(ev == event->ev);

time_t now = time(NULL); const or_options_t *options = get_options(); @@ -74,7 +71,7 @@ periodic_event_dispatch(evutil_socket_t fd, short what, void *data) // log_debug(LD_GENERAL, "Scheduling %s for %d seconds", event->name, // next_interval); struct timeval tv = { next_interval , 0 }; - event_add(event->ev, &tv); + mainloop_event_schedule(ev, &tv); }

/** Schedules <b>event</b> to run as soon as possible from now. */ @@ -93,10 +90,8 @@ periodic_event_setup(periodic_event_item_t *event) tor_assert(0); }

- event->ev = tor_event_new(tor_libevent_get_base(), - -1, 0, - periodic_event_dispatch, - event); + event->ev = mainloop_event_new(periodic_event_dispatch, + event); tor_assert(event->ev); }

@@ -111,7 +106,7 @@ periodic_event_launch(periodic_event_item_t *event) }

// Initial dispatch - periodic_event_dispatch(-1, EV_TIMEOUT, event); + periodic_event_dispatch(event->ev, event); }

/** Release all storage associated with <b>event</b> */ @@ -120,7 +115,7 @@ periodic_event_destroy(periodic_event_item_t *event) { if (!event) return; - tor_event_free(event->ev); + mainloop_event_free(event->ev); event->last_action_time = 0; }

diff --git a/src/or/periodic.h b/src/or/periodic.h index 8baf3994e..285400b8b 100644 --- a/src/or/periodic.h +++ b/src/or/periodic.h @@ -14,13 +14,14 @@ typedef int (*periodic_event_helper_t)(time_t now, const or_options_t *options);

-struct event; +struct mainloop_event_t;

/** A single item for the periodic-events-function table. */ typedef struct periodic_event_item_t { periodic_event_helper_t fn; /**< The function to run the event */ time_t last_action_time; /**< The last time the function did something */ - struct event *ev; /**< Libevent callback we're using to implement this */ + struct mainloop_event_t *ev; /**< Libevent callback we're using to implement + * this */ const char *name; /**< Name of the function -- for debug */ } periodic_event_item_t;

diff --git a/src/or/scheduler.c b/src/or/scheduler.c index 382b3e3ca..da894294b 100644 --- a/src/or/scheduler.c +++ b/src/or/scheduler.c @@ -13,8 +13,6 @@ #define TOR_CHANNEL_INTERNAL_ #include "channeltls.h"

-#include <event2/event.h> - /** * \file scheduler.c * \brief Channel scheduling system: decides which channels should send and @@ -169,7 +167,7 @@ STATIC smartlist_t *channels_pending = NULL; * This event runs the scheduler from its callback, and is manually * activated whenever a channel enters open for writes/cells to send. */ -STATIC struct event *run_sched_ev = NULL; +STATIC struct mainloop_event_t *run_sched_ev = NULL;

static int have_logged_kist_suddenly_disabled = 0;

@@ -203,10 +201,9 @@ get_scheduler_type_string(scheduler_types_t type) * if any scheduling work was created during the event loop. */ static void -scheduler_evt_callback(evutil_socket_t fd, short events, void *arg) +scheduler_evt_callback(mainloop_event_t *event, void *arg) { - (void) fd; - (void) events; + (void) event; (void) arg;

log_debug(LD_SCHED, "Scheduler event callback called"); @@ -487,10 +484,7 @@ scheduler_free_all(void) log_debug(LD_SCHED, "Shutting down scheduler");

if (run_sched_ev) { - if (event_del(run_sched_ev) < 0) { - log_warn(LD_BUG, "Problem deleting run_sched_ev"); - } - tor_event_free(run_sched_ev); + mainloop_event_free(run_sched_ev); run_sched_ev = NULL; }

@@ -589,7 +583,7 @@ scheduler_ev_add(const struct timeval *next_run) { tor_assert(run_sched_ev); tor_assert(next_run); - if (BUG(event_add(run_sched_ev, next_run) < 0)) { + if (BUG(mainloop_event_schedule(run_sched_ev, next_run) < 0)) { log_warn(LD_SCHED, "Adding to libevent failed. Next run time was set to: " "%ld.%06ld", next_run->tv_sec, (long)next_run->tv_usec); return; @@ -598,10 +592,10 @@ scheduler_ev_add(const struct timeval *next_run)

/** Make the scheduler event active with the given flags. */ void -scheduler_ev_active(int flags) +scheduler_ev_active(void) { tor_assert(run_sched_ev); - event_active(run_sched_ev, flags, 1); + mainloop_event_activate(run_sched_ev); }

/* @@ -618,11 +612,10 @@ scheduler_init(void) IF_BUG_ONCE(!!run_sched_ev) { log_warn(LD_SCHED, "We should not already have a libevent scheduler event." "I'll clean the old one up, but this is odd."); - tor_event_free(run_sched_ev); + mainloop_event_free(run_sched_ev); run_sched_ev = NULL; } - run_sched_ev = tor_event_new(tor_libevent_get_base(), -1, - 0, scheduler_evt_callback, NULL); + run_sched_ev = mainloop_event_new(scheduler_evt_callback, NULL); channels_pending = smartlist_new();

set_scheduler(); diff --git a/src/or/scheduler.h b/src/or/scheduler.h index aeba9e2b7..08b02e286 100644 --- a/src/or/scheduler.h +++ b/src/or/scheduler.h @@ -155,12 +155,12 @@ void scheduler_bug_occurred(const channel_t *chan); smartlist_t *get_channels_pending(void); MOCK_DECL(int, scheduler_compare_channels, (const void *c1_v, const void *c2_v)); -void scheduler_ev_active(int flags); +void scheduler_ev_active(void); void scheduler_ev_add(const struct timeval *next_run);

#ifdef TOR_UNIT_TESTS extern smartlist_t *channels_pending; -extern struct event *run_sched_ev; +extern struct mainloop_event_t *run_sched_ev; extern const scheduler_t *the_scheduler; void scheduler_touch_channel(channel_t *chan); #endif /* defined(TOR_UNIT_TESTS) */ diff --git a/src/or/scheduler_kist.c b/src/or/scheduler_kist.c index 6d6490077..c6e9b72c4 100644 --- a/src/or/scheduler_kist.c +++ b/src/or/scheduler_kist.c @@ -3,8 +3,6 @@

#define SCHEDULER_KIST_PRIVATE

-#include <event2/event.h> - #include "or.h" #include "buffers.h" #include "config.h" @@ -553,7 +551,7 @@ kist_scheduler_schedule(void) /* Re-adding an event reschedules it. It does not duplicate it. */ scheduler_ev_add(&next_run); } else { - scheduler_ev_active(EV_TIMEOUT); + scheduler_ev_active(); } }

diff --git a/src/or/scheduler_vanilla.c b/src/or/scheduler_vanilla.c index 7a83b9da1..b674d8256 100644 --- a/src/or/scheduler_vanilla.c +++ b/src/or/scheduler_vanilla.c @@ -1,8 +1,6 @@ /* Copyright (c) 2017, The Tor Project, Inc. */ /* See LICENSE for licensing information */

-#include <event2/event.h> - #include "or.h" #include "config.h" #define TOR_CHANNEL_INTERNAL_ @@ -42,7 +40,7 @@ vanilla_scheduler_schedule(void) }

/* Activate our event so it can process channels. */ - scheduler_ev_active(EV_TIMEOUT); + scheduler_ev_active(); }

static void