Merge branch 'maint-0.3.4' into release-0.3.4

3 files changed, 13 insertions, 10 deletions

diff --git a/changes/bug27003 b/changes/bug27003
new file mode 100644
index 0000000000..4f2045afc7
--- /dev/null
+++ b/changes/bug27003
@@ -0,0 +1,6 @@
+  o Major bugfixes (event scheduler):
+    - When we enable a periodic event, schedule it in the event loop
+      rather than running it immediately. Previously, we would re-run
+      periodic events immediately in the middle of (for example)
+      changing our options, with unpredictable effects.  Fixes bug
+      27003; bugfix on 0.3.4.1-alpha.
diff --git a/src/or/periodic.c b/src/or/periodic.c
index 92fa677f8f..9470376d06 100644
--- a/src/or/periodic.c
+++ b/src/or/periodic.c
@@ -140,8 +140,8 @@ periodic_event_destroy(periodic_event_item_t *event)
   event->last_action_time = 0;
 }
 
-/** Enable the given event which means the event is launched and then the
- * event's enabled flag is set. This can be called for an event that is
+/** Enable the given event by setting its "enabled" flag and scheduling it to
+ * run immediately in the event loop. This can be called for an event that is
  * already enabled. */
 void
 periodic_event_enable(periodic_event_item_t *event)
@@ -152,7 +152,9 @@ periodic_event_enable(periodic_event_item_t *event)
     return;
   }
 
-  periodic_event_launch(event);
+  tor_assert(event->ev);
+  event->enabled = 1;
+  mainloop_event_activate(event->ev);
 }
 
 /** Disable the given event which means the event is destroyed and then the
@@ -169,4 +171,3 @@ periodic_event_disable(periodic_event_item_t *event)
   mainloop_event_cancel(event->ev);
   event->enabled = 0;
 }
-
diff --git a/src/test/test_periodic_event.c b/src/test/test_periodic_event.c
index 34689b64f4..f159c4f83a 100644
--- a/src/test/test_periodic_event.c
+++ b/src/test/test_periodic_event.c
@@ -106,11 +106,11 @@ test_pe_launch(void *arg)
     periodic_event_item_t *item = &periodic_events[i];
     if (item->roles & PERIODIC_EVENT_ROLE_CLIENT) {
       tt_int_op(periodic_event_is_enabled(item), OP_EQ, 1);
-      tt_u64_op(item->last_action_time, OP_NE, 0);
     } else {
       tt_int_op(periodic_event_is_enabled(item), OP_EQ, 0);
-      tt_u64_op(item->last_action_time, OP_EQ, 0);
     }
+    // enabled or not, the event has not yet been run.
+    tt_u64_op(item->last_action_time, OP_EQ, 0);
   }
 
   /* Remove Client but become a Relay. */
@@ -127,12 +127,9 @@ test_pe_launch(void *arg)
     /* Only Client role should be disabled. */
     if (item->roles == PERIODIC_EVENT_ROLE_CLIENT) {
       tt_int_op(periodic_event_is_enabled(item), OP_EQ, 0);
-      /* Was previously enabled so they should never be to 0. */
-      tt_u64_op(item->last_action_time, OP_NE, 0);
     }
     if (item->roles & PERIODIC_EVENT_ROLE_RELAY) {
       tt_int_op(periodic_event_is_enabled(item), OP_EQ, 1);
-      tt_u64_op(item->last_action_time, OP_NE, 0);
     }
     /* Non Relay role should be disabled, except for Dirserver. */
     if (!(item->roles & roles)) {
@@ -330,4 +327,3 @@ struct testcase_t periodic_event_tests[] = {
 
   END_OF_TESTCASES
 };
-