[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

[tor-commits] [tlsdate/master] Refactor event loop.



commit aa04c0126a590fc9646d491151bcbfeed34ba693
Author: elly <elly@xxxxxxxxxxxxxx>
Date:   Mon Jun 24 15:36:01 2013 -0400

    Refactor event loop.
    
    Refactor the event loop to be modular and testable. Also, add support for
    detecting corruption of the realtime clock, as can be caused by suspend/resume
    cycles without an rtc battery. The event loop is now driven by a tree of events,
    which are either sources (currently suspend/resume events, periodic events, and
    network route events) or composite events.
    
    Signed-off-by: Elly Fong-Jones <elly@xxxxxxxxxxxxxx>
---
 CHANGELOG               |    2 +
 Makefile.am             |    2 +-
 run-tests               |    4 +-
 src/event-unittest.c    |   48 ++++++++
 src/event.c             |  295 ++++++++++++++++++++++++++++++++++++++++++++++
 src/event.h             |   21 ++++
 src/include.am          |   10 +-
 src/tlsdate.h           |    4 +
 src/tlsdated-unittest.c |   80 +++++++++++++
 src/tlsdated.c          |  300 +++++++++++++++++++++++++----------------------
 src/util.c              |  187 +++++++++++++++++++++++++++++
 src/util.h              |   19 +++
 tests/common.sh         |    2 +-
 13 files changed, 831 insertions(+), 143 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index e7c89f1..34879c5 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -17,6 +17,8 @@
   Add DragonFly BSD 3.3 support
   Refactored subprocess watching.
   Added integration tests. Run with ./run-tests
+  Refactored event loop.
+  Added suspend/resume RTC corruption detection.
 0.0.6 Mon 18 Feb, 2013
   Ensure that tlsdate compiles with g++ by explicit casting rather than
   implicit casting by whatever compiler is compiling tlsdate.
diff --git a/Makefile.am b/Makefile.am
index 5c2c845..cd5879b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -19,7 +19,7 @@ if !TARGET_OSX
 # GNU style is "make check", this will make check and test work
 TESTS+= src/conf_unittest src/proxy-bio_unittest
 if TARGET_LINUX
-TESTS+= src/tlsdated_unittest
+TESTS+= src/tlsdated_unittest src/event_unittest
 endif
 test: check
 endif
diff --git a/run-tests b/run-tests
index 693fa59..e3b4e09 100755
--- a/run-tests
+++ b/run-tests
@@ -5,9 +5,9 @@ run_test() {
 	if [ -r "$1"/tlsdated-flags ]; then
 		flags=$(cat "$1"/tlsdated-flags | sed "s/@TESTDIR@/$1/g")
 	elif [ -r "$1"/test.conf ]; then
-		flags="-w -p -r -l -s -f $1/test.conf"
+		flags="-w -p -r -l -s -f $1/test.conf -v"
 	else
-		flags="-w -p -r -l -s -f test.conf"
+		flags="-w -p -r -l -s -f test.conf -v"
 	fi
 	# flags are deliberately unquoted here so that they'll be interpolated
 	timeout 5 src/tlsdated $flags -- "$1"/subproc.sh <"$1"/input >"$1"/run-output \
diff --git a/src/event-unittest.c b/src/event-unittest.c
new file mode 100644
index 0000000..a97f941
--- /dev/null
+++ b/src/event-unittest.c
@@ -0,0 +1,48 @@
+#include "src/event.h"
+#include "src/test_harness.h"
+
+#include <sys/time.h>
+
+TEST(every) {
+	struct event *e = event_every(3);
+	time_t then = time(NULL);
+	time_t now;
+	EXPECT_EQ(1, event_wait(e));
+	now = time(NULL);
+	EXPECT_GT(now, then + 2);
+	event_free(e);
+}
+
+TEST(fdread) {
+	int fds[2];
+	struct event *e;
+	char z = '0';
+
+	ASSERT_EQ(0, pipe(fds));
+	e = event_fdread(fds[0]);
+
+	write(fds[1], &z, 1);
+	EXPECT_EQ(1, event_wait(e));
+	event_free(e);
+}
+
+TEST(composite) {
+	struct event *e0 = event_every(2);
+	struct event *e1 = event_every(3);
+	struct event *ec = event_composite();
+	time_t then = time(NULL);
+	time_t now;
+
+	ASSERT_EQ(0, event_composite_add(ec, e0));
+	ASSERT_EQ(0, event_composite_add(ec, e1));
+	EXPECT_EQ(1, event_wait(ec));
+	EXPECT_EQ(1, event_wait(ec));
+	EXPECT_EQ(1, event_wait(ec));
+	now = time(NULL);
+	EXPECT_EQ(now, then + 4);
+	event_free(ec);
+	event_free(e1);
+	event_free(e0);
+}
+
+TEST_HARNESS_MAIN
diff --git a/src/event.c b/src/event.c
new file mode 100644
index 0000000..3b2fedb
--- /dev/null
+++ b/src/event.c
@@ -0,0 +1,295 @@
+#include <assert.h>
+#include <signal.h>
+#include <stdlib.h>
+#include <sys/select.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <time.h>
+#include <unistd.h>
+
+#include "routeup.h"
+
+struct event {
+  const char *name;
+  int (*fd)(struct event *);
+  int (*wait)(struct event *);
+  void (*free)(struct event *);
+};
+
+struct event_fd {
+  struct event event;
+  int fd;
+  pid_t pid;
+};
+
+struct event_composite {
+  struct event event;
+  struct event **children;
+  size_t nchildren;
+};
+
+static int _subproc_fd(struct event *e)
+{
+  struct event_fd *efd = (struct event_fd *)e;
+  return efd->fd;
+}
+
+static int _subproc_wait(struct event *e)
+{
+  struct event_fd *efd = (struct event_fd *)e;
+  char buf;
+  if (read(_subproc_fd(e), &buf, 1) == 1)
+    return 1;
+  /* fd is broken...? */
+  close(efd->fd);
+  efd->fd = -1;
+  return -1;
+}
+
+static const char kZero = '0';
+
+static void _subproc_signal(int fd)
+{
+  ssize_t _unused = write(fd, &kZero, 1);
+  (void)_unused;
+}
+
+static void _subproc_free(struct event *e)
+{
+  struct event_fd *efd = (struct event_fd *)e;
+  if (efd->pid > 0) {
+    kill(efd->pid, SIGKILL);
+    waitpid(efd->pid, NULL, 0);
+  }
+  close(efd->fd);
+  free(efd);
+}
+
+static struct event *_event_subproc(void (*func)(void *, int), void *arg)
+{
+  struct event_fd *ev = malloc(sizeof *ev);
+  int fds[2];
+
+  if (!ev)
+    return NULL;
+
+  ev->event.fd = _subproc_fd;
+  ev->event.wait = _subproc_wait;
+  ev->event.free = _subproc_free;
+
+  if (pipe(fds))
+  {
+    free(ev);
+    return NULL;
+  }
+
+  ev->pid = fork();
+  if (ev->pid < 0)
+  {
+    close(fds[0]);
+    close(fds[1]);
+    free(ev);
+    return NULL;
+  }
+  else if (ev->pid == 0)
+  {
+    close(fds[0]);
+    func(arg, fds[1]);
+    exit(0);
+  }
+
+  close(fds[1]);
+  ev->fd = fds[0];
+
+  return &ev->event;
+}
+
+static void _routeup(void *arg, int fd)
+{
+  struct routeup *rtup = arg;
+  while (!routeup_once(rtup, 0))
+    _subproc_signal(fd);
+}
+
+struct event *event_routeup()
+{
+  struct routeup *rtup = malloc(sizeof *rtup);
+  struct event *e;
+  if (!rtup)
+    return NULL;
+  if (!routeup_setup(rtup))
+  {
+    free(rtup);
+    return NULL;
+  }
+  e = _event_subproc(_routeup, (void*)rtup);
+  /* free it in the parent only */
+  routeup_teardown(rtup);
+  if (e)
+    e->name = "routeup";
+  return e;
+}
+
+static void _every(void *arg, int fd)
+{
+  int delay = *(int*)arg;
+  while (1)
+  {
+    sleep(delay);
+    _subproc_signal(fd);
+  }
+}
+
+struct event *event_every(int seconds)
+{
+  struct event *e = _event_subproc(_every, (void *)&seconds);
+  e->name = "every";
+  return e;
+}
+
+static void _suspend(void *arg, int fd)
+{
+  while (1)
+  {
+    static const int kInterval = 60;
+    static const int kThreshold = 3;
+    time_t then = time(NULL);
+    time_t now;
+    sleep(kInterval);
+    now = time(NULL);
+    if (abs(now - then - kInterval) > kThreshold)
+      _subproc_signal(fd);
+  }
+}
+
+struct event *event_suspend()
+{
+  struct event *e = _event_subproc(_suspend, NULL);
+  e->name = "suspend";
+  return e;
+}
+
+struct event *event_fdread(int fd)
+{
+  struct event_fd *efd = malloc(sizeof *efd);
+  efd->event.name = "fdread";
+  efd->event.fd = _subproc_fd;
+  efd->event.wait = _subproc_wait;
+  efd->event.free = _subproc_free;
+  efd->fd = fd;
+  return &efd->event;
+}
+
+int event_wait(struct event *e)
+{
+  assert(e->wait);
+  return e->wait(e);
+}
+
+void event_free(struct event *e)
+{
+  assert(e->free);
+  return e->free(e);
+}
+
+static int _composite_fd(struct event *e)
+{
+  return -1;
+}
+
+static int _composite_wait(struct event *e)
+{
+  struct event_composite *ec = (struct event_composite *)e;
+  fd_set fds;
+  size_t i;
+  int n;
+  int maxfd = -1;
+
+  FD_ZERO(&fds);
+  for (i = 0; i < ec->nchildren; i++)
+  {
+    struct event *child = ec->children[i];
+    int fd = child->fd(child);
+    if (fd != -1)
+      FD_SET(child->fd(child), &fds);
+    if (fd > maxfd)
+      maxfd = fd;
+  }
+
+  n = select(maxfd + 1, &fds, NULL, NULL, NULL);
+  if (n < 0)
+    return -1;
+  for (i = 0; i < ec->nchildren; i++)
+  {
+    struct event *child = ec->children[i];
+    int fd = child->fd(child);
+    if (FD_ISSET(fd, &fds))
+    {
+      /* won't block, but clears the event */
+      int r = child->wait(child);
+      if (r)
+        /* either error or success, but not 'no event' */
+        return r;
+    }
+  }
+
+  return 0;
+}
+
+static void _composite_free(struct event *e)
+{
+  struct event_composite *ec = (struct event_composite *)e;
+  free(ec->children);
+  free(ec);
+}
+
+struct event *event_composite(void)
+{
+  struct event_composite *ec = malloc(sizeof *ec);
+  if (!ec)
+    return NULL;
+  ec->event.name = "composite";
+  ec->event.fd = _composite_fd;
+  ec->event.wait = _composite_wait;
+  ec->event.free = _composite_free;
+  ec->children = NULL;
+  ec->nchildren = 0;
+  return &ec->event;
+}
+
+int event_composite_add(struct event *comp, struct event *e)
+{
+  struct event_composite *ec = (struct event_composite *)comp;
+  size_t i;
+  size_t nsz;
+  struct event **nch;
+  for (i = 0; i < ec->nchildren; i++)
+  {
+    if (ec->children[i])
+      continue;
+    ec->children[i] = e;
+    return 0;
+  }
+  nsz = ec->nchildren + 1;
+  nch = realloc(ec->children, nsz * sizeof(struct event *));
+  if (!nch)
+    return 1;
+  ec->nchildren = nsz;
+  ec->children = nch;
+  ec->children[nsz - 1] = e;
+  return 0;
+}
+
+int event_composite_del(struct event *comp, struct event *e)
+{
+  struct event_composite *ec = (struct event_composite *)comp;
+  size_t i;
+  for (i = 0; i < ec->nchildren; i++)
+  {
+    if (ec->children[i] != e)
+      continue;
+    ec->children[i] = NULL;
+    return 0;
+  }
+  return 1;
+}
diff --git a/src/event.h b/src/event.h
new file mode 100644
index 0000000..ce4a535
--- /dev/null
+++ b/src/event.h
@@ -0,0 +1,21 @@
+/* event.h - event sources */
+
+#ifndef EVENT_H
+#define EVENT_H
+
+struct event;
+
+extern struct event *event_fdread(int fd);
+extern struct event *event_routeup(void);
+extern struct event *event_suspend(void);
+extern struct event *event_every(int seconds);
+
+extern struct event *event_composite(void);
+extern int event_composite_add(struct event *comp, struct event *e);
+extern int event_composite_del(struct event *comp, struct event *e);
+
+extern int event_wait(struct event *e);
+
+extern void event_free(struct event *e);
+
+#endif /* !EVENT_H */
diff --git a/src/include.am b/src/include.am
index f724f39..953bcd6 100644
--- a/src/include.am
+++ b/src/include.am
@@ -183,6 +183,7 @@ src_tlsdated_SOURCES+= src/common/android.c
 endif
 
 src_tlsdated_SOURCES+= src/conf.c
+src_tlsdated_SOURCES+= src/event.c
 src_tlsdated_SOURCES+= src/routeup.c
 src_tlsdated_SOURCES+= src/tlsdated.c
 src_tlsdated_SOURCES+= src/util.c
@@ -192,7 +193,8 @@ src_tlsdate_dbus_announce_LDADD = @DBUS_LIBS@
 src_tlsdate_dbus_announce_SOURCES = src/tlsdate-dbus-announce.c
 
 src_tlsdated_unittest_LDADD = @SSL_LIBS@
-src_tlsdated_unittest_SOURCES = src/routeup.c
+src_tlsdated_unittest_SOURCES = src/event.c
+src_tlsdated_unittest_SOURCES+= src/routeup.c
 src_tlsdated_unittest_SOURCES+= src/tlsdated-unittest.c
 src_tlsdated_unittest_SOURCES+= src/util.c
 src_tlsdated_unittest_SOURCES+= src/tlsdated.c
@@ -424,6 +426,12 @@ endif
 endif
 endif
 
+src_event_unittest_SOURCES = src/event.c
+src_event_unittest_SOURCES+= src/event-unittest.c
+src_event_unittest_SOURCES+= src/routeup.c
+src_event_unittest_SOURCES+= src/util.c
+check_PROGRAMS+= src/event_unittest
+
 if !TARGET_OSX
 check_PROGRAMS+= src/test/proxy-override src/test/return-argc \
                  src/test/rotate src/test/sleep-wrap
diff --git a/src/tlsdate.h b/src/tlsdate.h
index 978396d..c1e64ac 100644
--- a/src/tlsdate.h
+++ b/src/tlsdate.h
@@ -11,6 +11,7 @@
 #define TLSDATE_H
 
 #include "src/configmake.h"
+#include <limits.h>
 #include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -84,6 +85,9 @@ struct opts {
   int leap;
 };
 
+char timestamp_path[PATH_MAX];
+
+void sync_hwclock (void *rtc_handle);
 int is_sane_time (time_t ts);
 int load_disk_timestamp (const char *path, time_t * t);
 void save_disk_timestamp (const char *path, time_t t);
diff --git a/src/tlsdated-unittest.c b/src/tlsdated-unittest.c
index 5baca13..a56e57e 100644
--- a/src/tlsdated-unittest.c
+++ b/src/tlsdated-unittest.c
@@ -14,6 +14,7 @@
 #include <fcntl.h>
 #include <limits.h>
 #include <stdlib.h>
+#include <sys/time.h>
 #include <unistd.h>
 
 FIXTURE(tempdir) {
@@ -195,6 +196,24 @@ TEST(proxy_override) {
   EXPECT_EQ(0, tlsdate(&opts, environ));
 }
 
+FIXTURE(mock_platform) {
+  struct platform platform;
+  struct platform *old_platform;
+};
+
+FIXTURE_SETUP(mock_platform) {
+  self->old_platform = platform;
+  self->platform.rtc_open = NULL;
+  self->platform.rtc_write = NULL;
+  self->platform.rtc_read = NULL;
+  self->platform.rtc_close = NULL;
+  platform = &self->platform;
+}
+
+FIXTURE_TEARDOWN(mock_platform) {
+  platform = self->old_platform;
+}
+
 TEST(tlsdate_args) {
   struct source s1 = {
     .next = NULL,
@@ -213,4 +232,65 @@ TEST(tlsdate_args) {
   EXPECT_EQ(9, tlsdate(&opts, environ));
 }
 
+/*
+ * The stuff below this line is ugly. For a lot of these mock functions, we want
+ * to smuggle some state (our success) back to the caller, but there's no angle
+ * for that, so we're basically stuck with some static variables to store
+ * expectations and successes/failures. This could also be done with nested
+ * functions, but only gcc supports them.
+ */
+static const time_t sync_hwclock_expected = 12345678;
+
+static int sync_hwclock_time_get(struct timeval *tv) {
+  tv->tv_sec = sync_hwclock_expected;
+  tv->tv_usec = 0;
+  return 0;
+}
+
+static int sync_hwclock_rtc_write(void *handle, const struct timeval *tv) {
+  *(int *)handle = tv->tv_sec == sync_hwclock_expected;
+  return 0;
+}
+
+TEST_F(mock_platform, sync_hwclock) {
+  int ok = 0;
+  void *fake_handle = (void *)&ok;
+  self->platform.time_get = sync_hwclock_time_get;
+  self->platform.rtc_write = sync_hwclock_rtc_write;
+  sync_hwclock(fake_handle);
+  ASSERT_EQ(ok, 1);
+}
+
+static const time_t sync_and_save_expected = 12345678;
+
+static int sync_and_save_time_get(struct timeval *tv) {
+  tv->tv_sec = sync_and_save_expected;
+  tv->tv_usec = 0;
+  return 0;
+}
+
+static int sync_and_save_rtc_write(void *handle, const struct timeval *tv) {
+  *(int *)handle += tv->tv_sec == sync_and_save_expected;
+  return 0;
+}
+
+static int sync_and_save_file_write_ok = 0;
+
+static int sync_and_save_file_write(const char *path, void *buf, size_t sz) {
+  if (!strcmp(path, timestamp_path))
+    sync_and_save_file_write_ok++;
+  return 0;
+}
+
+TEST_F(mock_platform, sync_and_save) {
+  int nosave_ok = 0;
+  self->platform.time_get = sync_and_save_time_get;
+  self->platform.rtc_write = sync_and_save_rtc_write;
+  self->platform.file_write = sync_and_save_file_write;
+  sync_and_save(&sync_and_save_file_write_ok, 1);
+  ASSERT_EQ(sync_and_save_file_write_ok, 2);
+  sync_and_save(&nosave_ok, 0);
+  ASSERT_EQ(nosave_ok, 1);
+}
+
 TEST_HARNESS_MAIN
diff --git a/src/tlsdated.c b/src/tlsdated.c
index 7816223..0f6759b 100644
--- a/src/tlsdated.c
+++ b/src/tlsdated.c
@@ -38,12 +38,12 @@
 #endif
 
 #include "src/conf.h"
+#include "src/event.h"
 #include "src/routeup.h"
 #include "src/util.h"
 #include "src/tlsdate.h"
 
 const char *kCacheDir = DEFAULT_DAEMON_CACHEDIR;
-const char *kTempSuffix = DEFAULT_DAEMON_TMPSUFFIX;
 
 int
 is_sane_time (time_t ts)
@@ -145,25 +145,15 @@ tlsdate (struct opts *opts, char *envp[])
 int
 load_disk_timestamp (const char *path, time_t * t)
 {
-  int fd = open (path, O_RDONLY | O_NOFOLLOW);
   time_t tmpt;
-  if (fd < 0)
-    {
-      perror ("Can't open %s for reading", path);
-      return -1;
-    }
-  if (read (fd, &tmpt, sizeof (tmpt)) != sizeof (tmpt))
-    {
-      perror ("Can't read seconds from %s", path);
-      close (fd);
-      return -1;
-    }
-  close (fd);
-  if (!is_sane_time (tmpt))
-    {
-      perror ("Timevalue not sane: %lu", tmpt);
-      return -1;
-    }
+  if (platform->file_read(path, &tmpt, sizeof(tmpt))) {
+    info("can't load time file");
+    return -1;
+  }
+  if (!is_sane_time(tmpt)) {
+    info("time %lu not sane", tmpt);
+    return -1;
+  }
   *t = tmpt;
   return 0;
 }
@@ -172,34 +162,8 @@ load_disk_timestamp (const char *path, time_t * t)
 void
 save_disk_timestamp (const char *path, time_t t)
 {
-  char tmp[PATH_MAX];
-  int fd;
-
-  if (snprintf (tmp, sizeof (tmp), "%s%s", path, kTempSuffix) >= sizeof (tmp))
-    {
-      pinfo ("Path %s too long to use", path);
-      exit (1);
-    }
-
-  if ((fd = open (tmp, O_WRONLY | O_CREAT | O_NOFOLLOW | O_TRUNC,
-      S_IRUSR | S_IWUSR)) < 0)
-    {
-      pinfo ("open failed");
-      return;
-    }
-  if (write (fd, &t, sizeof (t)) != sizeof (t))
-    {
-      pinfo ("write failed");
-      close (fd);
-      return;
-    }
-  if (close (fd))
-    {
-      pinfo ("close failed");
-      return;
-    }
-  if (rename (tmp, path))
-    pinfo ("rename failed");
+  if (platform->file_write(path, &t, sizeof(t)))
+    info("saving disk timestamp failed");
 }
 
 /*
@@ -210,38 +174,17 @@ save_disk_timestamp (const char *path, time_t t)
  * this function except try later.
  */
 void
-sync_hwclock (int fd)
+sync_hwclock (void *rtc_handle)
 {
   struct timeval tv;
-  struct tm *tm;
-  struct rtc_time rtctm;
-
-  if (gettimeofday (&tv, NULL))
-    {
-      pinfo ("gettimeofday() failed");
-      return;
-    }
-
-  tm = gmtime (&tv.tv_sec);
-
-  /* these structs are identical, but separately defined */
-  rtctm.tm_sec = tm->tm_sec;
-  rtctm.tm_min = tm->tm_min;
-  rtctm.tm_hour = tm->tm_hour;
-  rtctm.tm_mday = tm->tm_mday;
-  rtctm.tm_mon = tm->tm_mon;
-  rtctm.tm_year = tm->tm_year;
-  rtctm.tm_wday = tm->tm_wday;
-  rtctm.tm_yday = tm->tm_yday;
-  rtctm.tm_isdst = tm->tm_isdst;
-
-  if (ioctl (fd, RTC_SET_TIME, &rtctm))
-    {
-      pinfo ("ioctl(%d, RTC_SET_TIME, ...) failed", fd);
-      return;
-    }
+  if (platform->time_get(&tv))
+  {
+    pinfo("gettimeofday() failed");
+    return;
+  }
 
-  info ("synced rtc to sysclock");
+  if (platform->rtc_write(rtc_handle, &tv))
+    info("rtc_write() failed");
 }
 
 /*
@@ -274,14 +217,14 @@ wait_for_event (struct routeup *rtc, int should_netlink, int timeout)
 char timestamp_path[PATH_MAX];
 
 void
-sync_and_save (int hwclock_fd, int should_sync, int should_save)
+sync_and_save (void *hwclock_handle, int should_save)
 {
   struct timeval tv;
-  if (should_sync)
-    sync_hwclock (hwclock_fd);
+  if (hwclock_handle)
+    sync_hwclock (hwclock_handle);
   if (should_save)
     {
-      if (gettimeofday (&tv, NULL))
+      if (platform->time_get(&tv))
   pfatal ("gettimeofday() failed");
       save_disk_timestamp (timestamp_path, tv.tv_sec);
     }
@@ -320,6 +263,12 @@ add_jitter (int base, int jitter)
   return base + (abs(n) % (2 * jitter)) - jitter;
 }
 
+int
+calc_wait_time (const struct opts *opts)
+{
+  return add_jitter (opts->steady_state_interval, opts->jitter);
+}
+
 #ifdef TLSDATED_MAIN
 #ifdef HAVE_DBUS
 void
@@ -348,6 +297,7 @@ void
 sigterm_handler (int _unused)
 {
   struct timeval tv;
+  platform->pgrp_kill();
   if (gettimeofday (&tv, NULL))
     /* can't use stdio or syslog inside a sig handler */
     exit (2);
@@ -356,6 +306,13 @@ sigterm_handler (int _unused)
 }
 
 void
+sigterm_handler_nosave (int _unused)
+{
+  platform->pgrp_kill();
+  exit(0);
+}
+
+void
 usage (const char *progn)
 {
   printf ("Usage: %s [flags...] [--] [tlsdate command...]\n", progn);
@@ -608,15 +565,60 @@ check_conf(struct opts *opts)
            opts->jitter, opts->steady_state_interval);
 }
 
+struct tlsdated_state
+{
+  struct routeup rtc;
+  time_t last_success;
+  struct opts *opts;
+  char *envp[];
+};
+
+static time_t now(void)
+{
+  struct timeval tv;
+  if (!platform->time_get(&tv))
+    return 0;
+  return tv.tv_sec;
+}
+
+int tlsdate_retry(struct opts *opts, char *envp[])
+{
+  int backoff = opts->wait_between_tries;
+  int i;
+
+  for (i = 0; i < opts->max_tries; i++)
+  {
+    if (!tlsdate(opts, envp))
+      return 0;
+    if (backoff < 1)
+      fatal("backoff too small? %d", backoff);
+    sleep(backoff);
+    if (backoff < MAX_SANE_BACKOFF)
+      backoff *= 2;
+  }
+  return 1;
+}
+
 int API
 main (int argc, char *argv[], char *envp[])
 {
-  struct routeup rtc;
-  int hwclock_fd = -1;
+  void *hwclock_handle = NULL;
   time_t last_success = 0;
   struct opts opts;
-  int wait_time = 0;
   struct timeval tv = { 0, 0 };
+  int r;
+
+  struct event *routeup = NULL;
+  struct event *suspend = NULL;
+  struct event *periodic = NULL; /* XXX */
+  struct event *composite = event_composite();
+
+  if (platform->pgrp_enter())
+   pfatal("pgrp_enter() failed");
+
+  suspend = event_suspend();
+
+  event_composite_add(composite, suspend);
 
   set_conf_defaults(&opts);
   parse_argv(&opts, argc, argv);
@@ -624,6 +626,9 @@ main (int argc, char *argv[], char *envp[])
   load_conf(&opts);
   check_conf(&opts);
 
+  periodic = event_every(opts.steady_state_interval);
+  event_composite_add(composite, periodic);
+
   info ("started up, loaded config file");
 
   if (!opts.should_load_disk || load_disk_timestamp (timestamp_path, &tv.tv_sec))
@@ -636,49 +641,53 @@ main (int argc, char *argv[], char *envp[])
                               (char *) DEFAULT_PROXY);
 
   /* grab a handle to /dev/rtc for sync_hwclock() */
-  if (opts.should_sync_hwclock && (hwclock_fd = open (DEFAULT_RTC_DEVICE, O_RDONLY)) < 0)
-    {
-      pinfo ("can't open hwclock fd");
-      opts.should_sync_hwclock = 0;
-    }
+  if (opts.should_sync_hwclock && !(hwclock_handle = platform->rtc_open()))
+    pinfo ("can't open hwclock fd");
+
+  if (opts.should_netlink)
+    routeup = event_routeup();
+  else
+    routeup = event_fdread(STDIN_FILENO);
+  event_composite_add(composite, routeup);
 
-  /* set up a netlink context if we need one */
-  if (opts.should_netlink && routeup_setup (&rtc))
+  if (!routeup)
     pfatal ("Can't open netlink socket");
 
   if (!is_sane_time (time (NULL)))
-    {
-      /*
-       * If the time is before the build timestamp, we're probably on
-       * a system with a broken rtc. Try loading the timestamp off
-       * disk.
-       */
-      tv.tv_sec = RECENT_COMPILE_DATE;
-      if (opts.should_load_disk &&
-    load_disk_timestamp (timestamp_path, &tv.tv_sec))
-  pinfo ("can't load disk timestamp");
-      if (!opts.dry_run && settimeofday (&tv, NULL))
-  pfatal ("settimeofday() failed");
-      dbus_announce();
-      /*
-       * don't save here - we either just loaded this time or used the
-       * default time, and neither of those are good to save
-       */
-      sync_and_save (hwclock_fd, opts.should_sync_hwclock, 0);
-    }
+  {
+    /*
+     * If the time is before the build timestamp, we're probably on
+     * a system with a broken rtc. Try loading the timestamp off
+     * disk.
+     */
+    tv.tv_sec = RECENT_COMPILE_DATE;
+    if (opts.should_load_disk
+        && load_disk_timestamp (timestamp_path, &tv.tv_sec))
+      pinfo ("can't load disk timestamp");
+    if (!opts.dry_run && settimeofday (&tv, NULL))
+      pfatal ("settimeofday() failed");
+    dbus_announce();
+    /*
+     * don't save here - we either just loaded this time or used the
+     * default time, and neither of those are good to save
+     */
+    sync_and_save (hwclock_handle, 0);
+  }
 
-  /* register a signal handler to save time at shutdown */
   if (opts.should_save_disk)
     signal (SIGTERM, sigterm_handler);
+  else
+    signal (SIGTERM, sigterm_handler_nosave);
 
   /*
    * Try once right away. If we fail, wait for a route to appear, then try
    * for a while; repeat whenever another route appears. Try until we
    * succeed.
    */
-  if (!tlsdate (&opts, envp)) {
+  if (!tlsdate (&opts, envp))
+  {
     last_success = time (NULL);
-    sync_and_save (hwclock_fd, opts.should_sync_hwclock, opts.should_save_disk);
+    sync_and_save (hwclock_handle, opts.should_save_disk);
     dbus_announce();
   }
 
@@ -688,36 +697,51 @@ main (int argc, char *argv[], char *envp[])
    * this should handle cases like a VPN being brought up and down
    * periodically.
    */
-  wait_time = add_jitter(opts.steady_state_interval, opts.jitter);
-  while (wait_for_event (&rtc, opts.should_netlink, wait_time) >= 0)
+
+  /*
+   * The event loop.
+   * When we start up, we may have no time fix at all; we'll call this "active
+   * mode", where we are actively looking for opportunities to get a time fix.
+   * Once we get a time fix, we go into "passive mode", where we're looking to
+   * prevent clock drift or rtc corruption. The difference between these two
+   * modes is whether we're aggressive about checking for time after network
+   * routes come up.
+   *
+   * To do this, we create some event sources:
+   *   e0 = event_routeup()
+   *   e1 = event_suspend()
+   *   e2 = event_every(interval)
+   * Then we create a couple of composite events:
+   *   active = event_anyof(3, e0, e1, e2)
+   * Whenever our wait for an event returns, we start checking (i.e., running
+   * tlsdate with exponential backoff until it succeeds). If checking succeeds
+   * and we were in active state, we go to passive state; otherwise we return to
+   * our previous state.
+   */
+
+  while ((r = event_wait(composite)))
+  {
+    if (r < 0)
     {
-      /*
-       * If a route just came up, run tlsdate; if it
-       * succeeded, then we're good and can keep time locally
-       * from now on.
-       */
-      int i;
-      int backoff = opts.wait_between_tries;
-      wait_time = add_jitter(opts.steady_state_interval, opts.jitter);
-      if (time (NULL) - last_success < opts.min_steady_state_interval)
-        continue;
-      for (i = 0; i < opts.max_tries && tlsdate (&opts, envp); ++i) {
-        if (backoff < 1)
-          fatal ("backoff too small? %d", backoff);
-        sleep (backoff);
-        if (backoff < MAX_SANE_BACKOFF)
-          backoff *= 2;
-      }
-      if (i != opts.max_tries)
-      {
-        last_success = time (NULL);
-        info ("tlsdate succeeded");
-        sync_and_save (hwclock_fd, opts.should_sync_hwclock, opts.should_save_disk);
-        dbus_announce();
-      }
+      info("event_wait() failed: %d", r);
+      continue;
     }
+    if (now() - last_success < opts.min_steady_state_interval)
+    {
+      info("too soon");
+      continue;
+    }
+    if (!tlsdate_retry(&opts, envp))
+    {
+      last_success = now();
+      info("tlsdate succeeded");
+      sync_and_save (hwclock_handle, opts.should_save_disk);
+      dbus_announce();
+    }
+  }
 
   info ("exiting");
+  platform->pgrp_kill();
   return 1;
 }
 #endif /* !TLSDATED_MAIN */
diff --git a/src/util.c b/src/util.c
index 30a0f44..fe5c370 100644
--- a/src/util.c
+++ b/src/util.c
@@ -6,19 +6,29 @@
  */
 
 #include "config.h"
+#include "tlsdate.h"
 
+#include <fcntl.h>
 #include <grp.h>
+#include <limits.h>
+#include <linux/rtc.h>
 #include <pwd.h>
 #include <signal.h>
 #include <stdarg.h>
 #include <stdio.h>
+#include <stdlib.h>
+#include <sys/ioctl.h>
+#include <sys/time.h>
 #include <sys/types.h>
 #include <sys/wait.h>
 #include <syslog.h>
+#include <time.h>
 #include <unistd.h>
 
 #include "src/util.h"
 
+const char *kTempSuffix = DEFAULT_DAEMON_TMPSUFFIX;
+
 /** helper function to print message and die */
 void
 die (const char *fmt, ...)
@@ -132,3 +142,180 @@ wait_with_timeout(int *status, int timeout_secs)
   *status = st;
   return exited;
 }
+
+struct rtc_handle
+{
+	int fd;
+};
+
+void *rtc_open()
+{
+	struct rtc_handle *h = malloc(sizeof *h);
+	h->fd = open(DEFAULT_RTC_DEVICE, O_RDONLY);
+	if (h->fd < 0)
+  {
+		pinfo("can't open rtc");
+		free(h);
+		return NULL;
+	}
+	return h;
+}
+
+int rtc_write(void *handle, const struct timeval *tv)
+{
+  struct tm tmr;
+  struct tm *tm;
+  struct rtc_time rtctm;
+  int fd = ((struct rtc_handle *)handle)->fd;
+
+  tm = gmtime_r (&tv->tv_sec, &tmr);
+
+  /* these structs are identical, but separately defined */
+  rtctm.tm_sec = tm->tm_sec;
+  rtctm.tm_min = tm->tm_min;
+  rtctm.tm_hour = tm->tm_hour;
+  rtctm.tm_mday = tm->tm_mday;
+  rtctm.tm_mon = tm->tm_mon;
+  rtctm.tm_year = tm->tm_year;
+  rtctm.tm_wday = tm->tm_wday;
+  rtctm.tm_yday = tm->tm_yday;
+  rtctm.tm_isdst = tm->tm_isdst;
+
+  if (ioctl (fd, RTC_SET_TIME, &rtctm))
+  {
+    pinfo ("ioctl(%d, RTC_SET_TIME, ...) failed", fd);
+    return 1;
+  }
+
+  info ("synced rtc to sysclock");
+  return 0;
+}
+
+int rtc_read(void *handle, struct timeval *tv)
+{
+  struct tm tm;
+  struct rtc_time rtctm;
+  int fd = ((struct rtc_handle *)handle)->fd;
+
+  if (ioctl (fd, RTC_RD_TIME, &rtctm))
+  {
+    pinfo ("ioctl(%d, RTC_RD_TIME, ...) failed", fd);
+    return 1;
+  }
+
+  tm.tm_sec = rtctm.tm_sec;
+  tm.tm_min = rtctm.tm_min;
+  tm.tm_hour = rtctm.tm_hour;
+  tm.tm_mday = rtctm.tm_mday;
+  tm.tm_mon = rtctm.tm_mon;
+  tm.tm_year = rtctm.tm_year;
+  tm.tm_wday = rtctm.tm_wday;
+  tm.tm_yday = rtctm.tm_yday;
+  tm.tm_isdst = rtctm.tm_isdst;
+
+  tv->tv_sec = mktime(&tm);
+  tv->tv_usec = 0;
+
+  return 0;
+}
+
+int rtc_close(void *handle)
+{
+	struct rtc_handle *h = handle;
+	close(h->fd);
+	free(h);
+	return 0;
+}
+
+int file_write(const char *path, void *buf, size_t sz)
+{
+	char tmp[PATH_MAX];
+	int oflags = O_WRONLY | O_CREAT | O_NOFOLLOW | O_TRUNC;
+	int perms = S_IRUSR | S_IWUSR;
+	int fd;
+
+	if (snprintf(tmp, sizeof(tmp), path, kTempSuffix) >= sizeof(tmp))
+  {
+		pinfo("path %s too long to use", path);
+		exit(1);
+	}
+
+	if ((fd = open(tmp, oflags, perms)) < 0)
+  {
+		pinfo("open(%s) failed", tmp);
+		return 1;
+	}
+
+	if (write(fd, buf, sz) != sz)
+  {
+		pinfo("write() failed");
+		close(fd);
+		return 1;
+	}
+
+	if (close(fd))
+  {
+		pinfo("close() failed");
+		return 1;
+	}
+
+	if (rename(tmp, path))
+  {
+		pinfo("rename() failed");
+		return 1;
+	}
+
+	return 0;
+}
+
+int file_read(const char *path, void *buf, size_t sz)
+{
+	int fd = open(path, O_RDONLY | O_NOFOLLOW);
+	if (fd < 0)
+  {
+		pinfo("open(%s) failed", path);
+		return 1;
+	}
+
+	if (read(fd, buf, sz) != sz)
+  {
+		pinfo("read() failed");
+		close(fd);
+		return 1;
+	}
+
+	return close(fd);
+}
+
+int time_get(struct timeval *tv)
+{
+	return gettimeofday(tv, NULL);
+}
+
+int pgrp_enter(void)
+{
+	return setpgid(0, 0);
+}
+
+int pgrp_kill(void)
+{
+	pid_t grp = getpgrp();
+	return kill(-grp, SIGKILL);
+}
+
+static struct platform default_platform = {
+	.rtc_open = rtc_open,
+	.rtc_write = rtc_write,
+	.rtc_read = rtc_read,
+	.rtc_close = rtc_close,
+
+	.file_write = file_write,
+	.file_read = file_read,
+
+	.time_get = time_get,
+
+	.pgrp_enter = pgrp_enter,
+	.pgrp_kill = pgrp_kill
+};
+
+struct platform *platform = &default_platform;
diff --git a/src/util.h b/src/util.h
index 2632933..4f63340 100644
--- a/src/util.h
+++ b/src/util.h
@@ -18,6 +18,8 @@
 
 #define API __attribute__((visibility("default")))
 
+extern const char *kTempSuffix;
+
 extern int verbose;
 void die (const char *fmt, ...);
 void verb (const char *fmt, ...);
@@ -41,4 +43,21 @@ void drop_privs_to (const char *user, const char *group);
  * ETIMEDOUT. */
 pid_t wait_with_timeout(int *status, int timeout_secs);
 
+struct platform {
+	void *(*rtc_open)(void);
+	int (*rtc_write)(void *handle, const struct timeval *tv);
+	int (*rtc_read)(void *handle, struct timeval *tv);
+	int (*rtc_close)(void *handle);
+
+	int (*file_write)(const char *path, void *buf, size_t sz);
+	int (*file_read)(const char *path, void *buf, size_t sz);
+
+	int (*time_get)(struct timeval *tv);
+
+	int (*pgrp_enter)(void);
+	int (*pgrp_kill)(void);
+};
+
+extern struct platform *platform;
+
 #endif /* !UTIL_H */
diff --git a/tests/common.sh b/tests/common.sh
index 96ba238..4283c01 100644
--- a/tests/common.sh
+++ b/tests/common.sh
@@ -1,7 +1,7 @@
 #!/bin/sh
 
 kill_tlsdated() {
-	kill -9 $PPID
+	kill -TERM $PPID
 }
 
 passed() {



_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits