nihilist/tor
1
0
Fork 0
mirror of https://gitlab.torproject.org/tpo/core/tor.git synced 2024-11-10 13:13:44 +01:00
Code Issues Packages Projects Releases Wiki Activity

When enabling periodic events, schedule but don't run them immediately.

Browse Source 
When we fixed 25939 in f7633c1fca, we
introduced a call to rescan_periodic_events() from inside the onion
service logic. But this meant that we could rescan the event list --
thereby running event callbacks! -- from inside the hidden service code.
This could cause us to run some of our event callbacks from an
inconsistent state, if we were in the middle of changing options.

A related bug (#25761) prevented us from rescanning our periodic
events as appropriate, but when we fixed THAT one, this bug reared
its ugly head.

The fix here is that "enabling" an event should cause us to run it
from the event loop, but not immediately from the point where we
enable it.

Fixes bug 27003; bugfix on 0.3.4.1-alpha.
This commit is contained in:
Nick Mathewson 2018-08-02 10:03:24 -04:00
parent 861d690018
commit 176999fd95
3 changed files with 13 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
changes/bug27003 Normal file
View File

@ -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.

9
src/or/periodic.c
View File

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

8
src/test/test_periodic_event.c
View File

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