Commit 290f5d33 authored by Juho Snellman's avatar Juho Snellman

Fix bug where timers scheduled from inside timers could end up in wrong slot

- During the timer processing the different wheels could have
  inconsistent views of time. The proper sequence is to first
  increment time in all wheels, and then process the timers from
  starting from the coarsest wheel and finishing with the most
  granular.
parent ae074796
...@@ -140,6 +140,29 @@ bool test_single_timer_hierarchy() { ...@@ -140,6 +140,29 @@ bool test_single_timer_hierarchy() {
return true; return true;
} }
bool test_reschedule_from_timer() {
typedef std::function<void()> Callback;
TimerWheel timers;
int count = 0;
TimerEvent<Callback> timer([&count] () { ++count; });
// For every slot in the outermost wheel, try scheduling a timer from
// a timer handler 258 ticks in the future. Then reschedule it in 257
// ticks. It should never actually trigger.
for (int i = 0; i < 256; ++i) {
TimerEvent<Callback> rescheduler([&timers, &timer] () { timers.schedule(&timer, 258); });
timers.schedule(&rescheduler, 1);
timers.advance(257);
EXPECT_INTEQ(count, 0);
}
// But once we stop rescheduling the timer, it'll trigger as intended.
timers.advance(2);
EXPECT_INTEQ(count, 1);
return true;
}
bool test_single_timer_random() { bool test_single_timer_random() {
typedef std::function<void()> Callback; typedef std::function<void()> Callback;
TimerWheel timers; TimerWheel timers;
...@@ -206,8 +229,8 @@ int main(void) { ...@@ -206,8 +229,8 @@ int main(void) {
TEST(test_single_timer_no_hierarchy); TEST(test_single_timer_no_hierarchy);
TEST(test_single_timer_hierarchy); TEST(test_single_timer_hierarchy);
TEST(test_single_timer_random); TEST(test_single_timer_random);
TEST(test_reschedule_from_timer);
TEST(test_timeout_method); TEST(test_timeout_method);
// Test canceling timer from within timer // Test canceling timer from within timer
// Test rescheduling timer from within timer
return ok ? 0 : 1; return ok ? 0 : 1;
} }
...@@ -127,6 +127,9 @@ public: ...@@ -127,6 +127,9 @@ public:
now_++; now_++;
size_t slot_index = now_ & MASK; size_t slot_index = now_ & MASK;
auto slot = &slots_[slot_index]; auto slot = &slots_[slot_index];
if (slot_index == 0 && up_) {
up_->advance(1);
}
while (slot->events()) { while (slot->events()) {
auto event = slot->pop_event(); auto event = slot->pop_event();
if (down_) { if (down_) {
...@@ -136,9 +139,6 @@ public: ...@@ -136,9 +139,6 @@ public:
event->execute(); event->execute();
} }
} }
if (slot_index == 0 && up_) {
up_->advance(1);
}
} }
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment