Require non-negative times for AOS timers
Linux doesn't like negative monotonic clock times, so disallow them.
In order to support this, LogReader now does not schedule events at
times known to be in the past (this should be a no-op for simulated
event loops, but for realtime replay with non-zero clock offsets will
avoid scheduling events too far in the past).
References: PRO-14846
Change-Id: Ieb4bf58c5e1083ff30c7df7bfddff356c95ba9ba
Signed-off-by: James Kuszmaul <james.kuszmaul@bluerivertech.com>
diff --git a/aos/events/epoll.cc b/aos/events/epoll.cc
index 8cb553b..71e3c9b 100644
--- a/aos/events/epoll.cc
+++ b/aos/events/epoll.cc
@@ -24,6 +24,7 @@
void TimerFd::SetTime(monotonic_clock::time_point start,
monotonic_clock::duration interval) {
+ CHECK_GE(start, monotonic_clock::epoch());
struct itimerspec new_value;
new_value.it_interval = ::aos::time::to_timespec(interval);
new_value.it_value = ::aos::time::to_timespec(start);
diff --git a/aos/events/event_loop.h b/aos/events/event_loop.h
index 3ecd93f..316ac29 100644
--- a/aos/events/event_loop.h
+++ b/aos/events/event_loop.h
@@ -462,6 +462,7 @@
// Timer should sleep until base, base + offset, base + offset * 2, ...
// If repeat_offset isn't set, the timer only expires once.
+ // base must be greater than or equal to zero.
virtual void Setup(monotonic_clock::time_point base,
monotonic_clock::duration repeat_offset =
::aos::monotonic_clock::zero()) = 0;
diff --git a/aos/events/event_loop_param_test.cc b/aos/events/event_loop_param_test.cc
index 2d7decc..0eeecf8 100644
--- a/aos/events/event_loop_param_test.cc
+++ b/aos/events/event_loop_param_test.cc
@@ -752,6 +752,14 @@
"/test/invalid");
}
+// Verify that setting up a timer before monotonic_clock::epoch() fails.
+TEST_P(AbstractEventLoopDeathTest, NegativeTimeTimer) {
+ auto loop = Make();
+ TimerHandler *time = loop->AddTimer([]() {});
+ EXPECT_DEATH(time->Setup(monotonic_clock::epoch() - std::chrono::seconds(1)),
+ "-1.000");
+}
+
// Verify that registering a watcher twice for "/test" fails.
TEST_P(AbstractEventLoopDeathTest, TwoWatcher) {
auto loop = Make();
diff --git a/aos/events/logging/log_reader.h b/aos/events/logging/log_reader.h
index 2c31e0f..1b87e9d 100644
--- a/aos/events/logging/log_reader.h
+++ b/aos/events/logging/log_reader.h
@@ -505,7 +505,8 @@
// Sets the next wakeup time on the replay callback.
void Setup(monotonic_clock::time_point next_time) {
- timer_handler_->Setup(next_time + clock_offset());
+ timer_handler_->Setup(
+ std::max(monotonic_now(), next_time + clock_offset()));
}
// Sends a buffer on the provided channel index.
diff --git a/aos/events/simulated_event_loop.cc b/aos/events/simulated_event_loop.cc
index e0ed7a0..33893ad 100644
--- a/aos/events/simulated_event_loop.cc
+++ b/aos/events/simulated_event_loop.cc
@@ -1140,6 +1140,7 @@
void SimulatedTimerHandler::Setup(monotonic_clock::time_point base,
monotonic_clock::duration repeat_offset) {
+ CHECK_GE(base, monotonic_clock::epoch());
// The allocations in here are due to infrastructure and don't count in the no
// mallocs in RT code.
ScopedNotRealtime nrt;
diff --git a/y2020/control_loops/drivetrain/localizer_test.cc b/y2020/control_loops/drivetrain/localizer_test.cc
index f0bcd88..91b5505 100644
--- a/y2020/control_loops/drivetrain/localizer_test.cc
+++ b/y2020/control_loops/drivetrain/localizer_test.cc
@@ -91,7 +91,7 @@
return locations;
}
-constexpr std::chrono::seconds kPiTimeOffset(-10);
+constexpr std::chrono::seconds kPiTimeOffset(10);
} // namespace
namespace chrono = std::chrono;