Allow a TimerHandler to cancel itself
The new test failed with ShmEventLoop before.
PhasedLoopHandlers have even bigger problems with this kind of usage,
but those aren't usually adjusted while running so it's not as big of a
deal.
Change-Id: I0715f9550fd099647ec0bf52372fbf9c078cc227
diff --git a/aos/events/event_loop_param_test.cc b/aos/events/event_loop_param_test.cc
index 979de74..248dcd3 100644
--- a/aos/events/event_loop_param_test.cc
+++ b/aos/events/event_loop_param_test.cc
@@ -1090,6 +1090,28 @@
EXPECT_EQ(iteration_list.size(), 3);
}
+// Verify that a timer can disable itself.
+//
+// TODO(Brian): Do something similar with phased loops, both with a quick
+// handler and a handler that would miss a cycle except it got deferred. Current
+// behavior doing that is a mess.
+TEST_P(AbstractEventLoopTest, TimerDisableSelf) {
+ auto loop = MakePrimary();
+
+ int count = 0;
+ aos::TimerHandler *test_timer;
+ test_timer = loop->AddTimer([&count, &test_timer]() {
+ ++count;
+ test_timer->Disable();
+ });
+
+ test_timer->Setup(loop->monotonic_now(), ::std::chrono::milliseconds(20));
+ EndEventLoop(loop.get(), ::std::chrono::milliseconds(80));
+ Run();
+
+ EXPECT_EQ(count, 1);
+}
+
// Verify that we can disable a timer during execution of another timer
// scheduled for the same time, with one ordering of creation for the timers.
//
diff --git a/aos/events/event_loop_tmpl.h b/aos/events/event_loop_tmpl.h
index 12953d1..9e16d20 100644
--- a/aos/events/event_loop_tmpl.h
+++ b/aos/events/event_loop_tmpl.h
@@ -259,7 +259,7 @@
.count();
timing_.handler_time.Add(handler_latency);
- // If the handler too too long so we blew by the previous deadline, we
+ // If the handler took too long so we blew by the previous deadline, we
// want to just try for the next deadline. Reschedule.
if (monotonic_end_time > phased_loop_.sleep_time()) {
Reschedule(schedule, monotonic_end_time);
diff --git a/aos/events/shm_event_loop.cc b/aos/events/shm_event_loop.cc
index 04d139f..6e152cc 100644
--- a/aos/events/shm_event_loop.cc
+++ b/aos/events/shm_event_loop.cc
@@ -554,10 +554,10 @@
CHECK_LE(length, static_cast<size_t>(channel()->max_size()))
<< ": Sent too big a message on "
<< configuration::CleanedChannelToString(channel());
- CHECK(lockless_queue_sender_.Send(reinterpret_cast<const char *>(msg), length,
- monotonic_remote_time, realtime_remote_time,
- remote_queue_index, &monotonic_sent_time_,
- &realtime_sent_time_, &sent_queue_index_))
+ CHECK(lockless_queue_sender_.Send(
+ reinterpret_cast<const char *>(msg), length, monotonic_remote_time,
+ realtime_remote_time, remote_queue_index, &monotonic_sent_time_,
+ &realtime_sent_time_, &sent_queue_index_))
<< ": Somebody wrote outside the buffer of their message on channel "
<< configuration::CleanedChannelToString(channel());
wake_upper_.Wakeup(event_loop()->priority());
@@ -667,12 +667,18 @@
void HandleEvent() {
CHECK(!event_.valid());
+ disabled_ = false;
const auto monotonic_now = Call(monotonic_clock::now, base_);
if (event_.valid()) {
// If someone called Setup inside Call, rescheduling is already taken care
// of. Bail.
return;
}
+ if (disabled_) {
+ // Somebody called Disable inside Call, so we don't want to reschedule.
+ // Bail.
+ return;
+ }
if (repeat_offset_ == chrono::seconds(0)) {
timerfd_.Disable();
@@ -708,6 +714,7 @@
void Disable() override {
shm_event_loop_->RemoveEvent(&event_);
timerfd_.Disable();
+ disabled_ = true;
}
private:
@@ -718,6 +725,10 @@
monotonic_clock::time_point base_;
monotonic_clock::duration repeat_offset_;
+
+ // Used to track if Disable() was called during the callback, so we know not
+ // to reschedule.
+ bool disabled_ = false;
};
// Adapter class to the timerfd and PhasedLoop.