Use Libevent 2.0's periodic timers where available.

These timers behave better with non-monotonic clocks than our old
ones, and also try harder to make once-per-second events get called
one second apart, rather than one-plus-epsilon seconds apart.

This fixes bug 943 for everybody using Libevent 2.0 or later.

4 files changed, 110 insertions, 24 deletions

diff --git a/changes/once_per_sec b/changes/once_per_sec
new file mode 100644
index 0000000000..abab6da0cd
--- /dev/null
+++ b/changes/once_per_sec
@@ -0,0 +1,4 @@
+ o Minor features
+   - Where available, use Libevent 2.0's periodic timers so that our
+     once-per-second cleanup code gets called even more closely to
+     once per second than it would otherwise.  Fix for bug 943.
diff --git a/src/common/compat_libevent.c b/src/common/compat_libevent.c
index 56ba3235b5..eaff7d7393 100644
--- a/src/common/compat_libevent.c
+++ b/src/common/compat_libevent.c
@@ -469,3 +469,82 @@ tor_check_libevent_header_compatibility(void)
 #endif
 }
 
+/*
+  If possible, we're going to try to use Libevent's periodic timer support,
+  since it does a pretty good job of making sure that periodic events get
+  called exactly M seconds apart, rather than starting each one exactly M
+  seconds after the time that the last one was run.
+ */
+#ifdef HAVE_EVENT2_EVENT_H
+#define HAVE_PERIODIC
+#define PERIODIC_FLAGS EV_PERSIST
+#else
+#define PERIODIC_FLAGS 0
+#endif
+
+/** Represents a timer that's run every N microseconds by Libevent. */
+struct periodic_timer_t {
+  /** Underlying event used to implement this periodic event. */
+  struct event *ev;
+  /** The callback we'll be invoking whenever the event triggers */
+  void (*cb)(struct periodic_timer_t *, void *);
+  /** User-supplied data for the callback */
+  void *data;
+#ifndef HAVE_PERIODIC
+  /** If Libevent doesn't know how to invoke events every N microseconds,
+   * we'll need to remember the timeout interval here. */
+  struct timeval tv;
+#endif
+};
+
+/** Libevent callback to implement a periodic event. */
+static void
+periodic_timer_cb(evutil_socket_t fd, short what, void *arg)
+{
+  periodic_timer_t *timer = arg;
+  (void) what;
+  (void) fd;
+#ifndef HAVE_PERIODIC
+  /** reschedule the event as needed. */
+  event_add(timer->ev, &timer->tv);
+#endif
+  timer->cb(timer, timer->data);
+}
+
+/** Create and schedule a new timer that will run every <b>tv</b> in
+ * the event loop of <b>base</b>.  When the timer fires, it will
+ * run the timer in <b>cb</b> with the user-supplied data in <b>data</b>. */
+periodic_timer_t *
+periodic_timer_new(struct event_base *base,
+                   const struct timeval *tv,
+                   void (*cb)(periodic_timer_t *timer, void *data),
+                   void *data)
+{
+  periodic_timer_t *timer;
+  tor_assert(base);
+  tor_assert(tv);
+  tor_assert(cb);
+  timer = tor_malloc_zero(sizeof(periodic_timer_t));
+  if (!(timer->ev = tor_event_new(base, -1, PERIODIC_FLAGS,
+                                  periodic_timer_cb, timer))) {
+    tor_free(timer);
+    return NULL;
+  }
+  timer->cb = cb;
+  timer->data = data;
+#ifndef HAVE_PERIODIC
+  memcpy(&timer->tv, tv, sizeof(struct timeval));
+#endif
+  event_add(timer->ev, tv);
+  return timer;
+}
+
+/** Stop and free a periodic timer */
+void
+periodic_timer_free(periodic_timer_t *timer)
+{
+  if (!timer)
+    return;
+  tor_event_free(timer->ev);
+  tor_free(timer);
+}
diff --git a/src/common/compat_libevent.h b/src/common/compat_libevent.h
index eedd9da43d..fdf5e0a18f 100644
--- a/src/common/compat_libevent.h
+++ b/src/common/compat_libevent.h
@@ -38,8 +38,14 @@ void tor_event_free(struct event *ev);
 #define tor_evdns_add_server_port evdns_add_server_port
 #endif
 
-/* XXXX022 If we can drop support for Libevent before 1.1, we can
- * do without this wrapper. */
+typedef struct periodic_timer_t periodic_timer_t;
+
+periodic_timer_t *periodic_timer_new(struct event_base *base,
+             const struct timeval *tv,
+             void (*cb)(periodic_timer_t *timer, void *data),
+             void *data);
+void periodic_timer_free(periodic_timer_t *);
+
 #ifdef HAVE_EVENT_BASE_LOOPEXIT
 #define tor_event_base_loopexit event_base_loopexit
 #else
diff --git a/src/or/main.c b/src/or/main.c
index 0c3e6d5425..0dcc45464a 100644
--- a/src/or/main.c
+++ b/src/or/main.c
@@ -33,7 +33,7 @@ static void dumpstats(int severity); /* log stats */
 static void conn_read_callback(int fd, short event, void *_conn);
 static void conn_write_callback(int fd, short event, void *_conn);
 static void signal_callback(int fd, short events, void *arg);
-static void second_elapsed_callback(int fd, short event, void *args);
+static void second_elapsed_callback(periodic_timer_t *timer, void *args);
 static int conn_close_if_marked(int i);
 static void connection_start_reading_from_linked_conn(connection_t *conn);
 static int connection_should_read_from_linked_conn(connection_t *conn);
@@ -1205,39 +1205,30 @@ run_scheduled_events(time_t now)
   }
 }
 
-/** Libevent timer: used to invoke second_elapsed_callback() once per
- * second. */
-static struct event *timeout_event = NULL;
+/** Timer: used to invoke second_elapsed_callback() once per second. */
+static periodic_timer_t *second_timer = NULL;
 /** Number of libevent errors in the last second: we die if we get too many. */
 static int n_libevent_errors = 0;
 
 /** Libevent callback: invoked once every second. */
 static void
-second_elapsed_callback(int fd, short event, void *args)
+second_elapsed_callback(periodic_timer_t *timer, void *arg)
 {
   /* XXXX This could be sensibly refactored into multiple callbacks, and we
    * could use Libevent's timers for this rather than checking the current
    * time against a bunch of timeouts every second. */
-  static struct timeval one_second;
   static time_t current_second = 0;
   time_t now;
   size_t bytes_written;
   size_t bytes_read;
   int seconds_elapsed;
   or_options_t *options = get_options();
-  (void)fd;
-  (void)event;
-  (void)args;
-  if (!timeout_event) {
-    timeout_event = tor_evtimer_new(tor_libevent_get_base(),
-                                    second_elapsed_callback, NULL);
-    one_second.tv_sec = 1;
-    one_second.tv_usec = 0;
-  }
+  (void)timer;
+  (void)arg;
 
   n_libevent_errors = 0;
 
-  /* log_fn(LOG_NOTICE, "Tick."); */
+  /* log_notice(LD_GENERAL, "Tick."); */
   now = time(NULL);
   update_approx_time(now);
 
@@ -1302,10 +1293,6 @@ second_elapsed_callback(int fd, short event, void *args)
   run_scheduled_events(now);
 
   current_second = now; /* remember which second it is, for next time */
-
-  if (event_add(timeout_event, &one_second))
-    log_err(LD_NET,
-            "Error from libevent when setting one-second timeout event");
 }
 
 #ifndef MS_WINDOWS
@@ -1492,7 +1479,17 @@ do_main_loop(void)
   }
 
   /* set up once-a-second callback. */
-  second_elapsed_callback(0,0,NULL);
+  if (! second_timer) {
+    struct timeval one_second;
+    one_second.tv_sec = 1;
+    one_second.tv_usec = 0;
+
+    second_timer = periodic_timer_new(tor_libevent_get_base(),
+                                      &one_second,
+                                      second_elapsed_callback,
+                                      NULL);
+    tor_assert(second_timer);
+  }
 
   for (;;) {
     if (nt_service_is_stopping())
@@ -2013,7 +2010,7 @@ tor_free_all(int postfork)
   smartlist_free(connection_array);
   smartlist_free(closeable_connection_lst);
   smartlist_free(active_linked_connection_lst);
-  tor_free(timeout_event);
+  periodic_timer_free(second_timer);
   if (!postfork) {
     release_lockfile();
   }