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