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

@@ -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)
 {

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
 

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)ev;
-  (void)what;
   (void)arg;
 
   timers_run_pending();
@@ -203,9 +200,8 @@ timers_initialize(void)
   monotime_init();
   monotime_get(&start_of_time);
 
-  struct event *timer_event;
+  mainloop_event_t *timer_event;
-  timer_event = tor_event_new(tor_libevent_get_base(),
+  timer_event = mainloop_event_new(libevent_timer_callback, NULL);
-                              -1, 0, 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) {

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,
+      flush_queued_events_event =
-                                              -1, 0, flush_queued_events_cb,
+        mainloop_event_new(flush_queued_events_cb, NULL);
-                                              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) event;
-  (void) what;
   (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;

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(),
+      mainloop_event_new(directory_all_unreachable_cb, NULL);
-                    -1, EV_READ, 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);

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,9 +90,7 @@ periodic_event_setup(periodic_event_item_t *event)
     tor_assert(0);
   }
 
-  event->ev = tor_event_new(tor_libevent_get_base(),
+  event->ev = mainloop_event_new(periodic_event_dispatch,
-                            -1, 0,
-                            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;
 }
 

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;
 

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) event;
-  (void) events;
   (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) {
+    mainloop_event_free(run_sched_ev);
-      log_warn(LD_BUG, "Problem deleting run_sched_ev");
-    }
-    tor_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,
+  run_sched_ev = mainloop_event_new(scheduler_evt_callback, NULL);
-                               0, scheduler_evt_callback, NULL);
   channels_pending = smartlist_new();
 
   set_scheduler();

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) */

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();
   }
 }
 

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