Clear Context in EventLoop at construction time
We were exposing uninitialized memory and hoping our users wouldn't try
to access it. This is an overly bold assumption.
Change-Id: I286fe5e932354581419059d128563c435b16071b
Signed-off-by: Austin Schuh <austin.schuh@bluerivertech.com>
diff --git a/aos/events/event_loop.cc b/aos/events/event_loop.cc
index df0390c..09a3834 100644
--- a/aos/events/event_loop.cc
+++ b/aos/events/event_loop.cc
@@ -89,6 +89,7 @@
context_.realtime_event_time = realtime_clock::min_time;
context_.realtime_remote_time = realtime_clock::min_time;
context_.queue_index = 0xffffffff;
+ context_.remote_queue_index = 0xffffffffu;
context_.size = 0;
context_.data = nullptr;
context_.buffer_index = -1;
@@ -600,6 +601,19 @@
return result;
}
+void EventLoop::ClearContext() {
+ context_.monotonic_event_time = monotonic_clock::min_time;
+ context_.monotonic_remote_time = monotonic_clock::min_time;
+ context_.realtime_event_time = realtime_clock::min_time;
+ context_.realtime_remote_time = realtime_clock::min_time;
+ context_.queue_index = 0xffffffffu;
+ context_.remote_queue_index = 0xffffffffu;
+ context_.size = 0u;
+ context_.data = nullptr;
+ context_.buffer_index = -1;
+ context_.source_boot_uuid = boot_uuid();
+}
+
void EventLoop::SetTimerContext(
monotonic_clock::time_point monotonic_event_time) {
context_.monotonic_event_time = monotonic_event_time;
@@ -607,6 +621,7 @@
context_.realtime_event_time = realtime_clock::min_time;
context_.realtime_remote_time = realtime_clock::min_time;
context_.queue_index = 0xffffffffu;
+ context_.remote_queue_index = 0xffffffffu;
context_.size = 0u;
context_.data = nullptr;
context_.buffer_index = -1;
diff --git a/aos/events/event_loop.h b/aos/events/event_loop.h
index 27f0c76..ff1183f 100644
--- a/aos/events/event_loop.h
+++ b/aos/events/event_loop.h
@@ -856,6 +856,8 @@
// Sets context_ for a timed event which is supposed to happen at the provided
// time.
void SetTimerContext(monotonic_clock::time_point monotonic_event_time);
+ // Clears context_ so it only has invalid times and elements in it.
+ void ClearContext();
private:
virtual pid_t GetTid() = 0;
diff --git a/aos/events/event_loop_param_test.cc b/aos/events/event_loop_param_test.cc
index 118b9ae..990cbe5 100644
--- a/aos/events/event_loop_param_test.cc
+++ b/aos/events/event_loop_param_test.cc
@@ -2562,6 +2562,17 @@
TEST_P(AbstractEventLoopTest, SetContextOnRun) {
auto loop = MakePrimary();
+ EXPECT_EQ(loop->context().monotonic_event_time, monotonic_clock::min_time);
+ EXPECT_EQ(loop->context().monotonic_remote_time, monotonic_clock::min_time);
+ EXPECT_EQ(loop->context().realtime_event_time, realtime_clock::min_time);
+ EXPECT_EQ(loop->context().realtime_remote_time, realtime_clock::min_time);
+ EXPECT_EQ(loop->context().source_boot_uuid, loop->boot_uuid());
+ EXPECT_EQ(loop->context().queue_index, 0xffffffffu);
+ EXPECT_EQ(loop->context().remote_queue_index, 0xffffffffu);
+ EXPECT_EQ(loop->context().size, 0u);
+ EXPECT_EQ(loop->context().data, nullptr);
+ EXPECT_EQ(loop->context().buffer_index, -1);
+
// We want to check that monotonic event time is before monotonic now
// called inside of callback, but after time point obtained callback.
aos::monotonic_clock::time_point monotonic_event_time_on_run;
@@ -2574,6 +2585,7 @@
EXPECT_EQ(loop->context().realtime_remote_time, realtime_clock::min_time);
EXPECT_EQ(loop->context().source_boot_uuid, loop->boot_uuid());
EXPECT_EQ(loop->context().queue_index, 0xffffffffu);
+ EXPECT_EQ(loop->context().remote_queue_index, 0xffffffffu);
EXPECT_EQ(loop->context().size, 0u);
EXPECT_EQ(loop->context().data, nullptr);
EXPECT_EQ(loop->context().buffer_index, -1);
@@ -2585,6 +2597,17 @@
loop->monotonic_now();
Run();
EXPECT_GE(monotonic_event_time_on_run, before_run_time);
+
+ EXPECT_EQ(loop->context().monotonic_event_time, monotonic_clock::min_time);
+ EXPECT_EQ(loop->context().monotonic_remote_time, monotonic_clock::min_time);
+ EXPECT_EQ(loop->context().realtime_event_time, realtime_clock::min_time);
+ EXPECT_EQ(loop->context().realtime_remote_time, realtime_clock::min_time);
+ EXPECT_EQ(loop->context().source_boot_uuid, loop->boot_uuid());
+ EXPECT_EQ(loop->context().queue_index, 0xffffffffu);
+ EXPECT_EQ(loop->context().remote_queue_index, 0xffffffffu);
+ EXPECT_EQ(loop->context().size, 0u);
+ EXPECT_EQ(loop->context().data, nullptr);
+ EXPECT_EQ(loop->context().buffer_index, -1);
}
// Tests that watchers fail when created on the wrong node.
diff --git a/aos/events/shm_event_loop.cc b/aos/events/shm_event_loop.cc
index 3c89a1c..b2502a7 100644
--- a/aos/events/shm_event_loop.cc
+++ b/aos/events/shm_event_loop.cc
@@ -227,6 +227,7 @@
name_(FLAGS_application_name),
node_(MaybeMyNode(configuration)) {
CHECK(IsInitialized()) << ": Need to initialize AOS first.";
+ ClearContext();
if (configuration->has_nodes()) {
CHECK(node_ != nullptr) << ": Couldn't find node in config.";
}
@@ -1185,6 +1186,7 @@
// the event loop so the book keeping matches. Do this in the thread that
// created the timing reporter.
timing_report_sender_.reset();
+ ClearContext();
}
void ShmEventLoop::Exit() { epoll_.Quit(); }
diff --git a/aos/events/simulated_event_loop.cc b/aos/events/simulated_event_loop.cc
index 83857b1..d2328fb 100644
--- a/aos/events/simulated_event_loop.cc
+++ b/aos/events/simulated_event_loop.cc
@@ -572,6 +572,7 @@
tid_(tid),
startup_tracker_(std::make_shared<StartupTracker>()),
options_(options) {
+ ClearContext();
startup_tracker_->loop = this;
scheduler_->ScheduleOnStartup([startup_tracker = startup_tracker_]() {
if (startup_tracker->loop) {
@@ -669,6 +670,7 @@
ScopedMarkRealtimeRestorer rt(runtime_realtime_priority() > 0);
SetTimerContext(monotonic_now());
on_run();
+ ClearContext();
});
}
@@ -931,6 +933,7 @@
ScopedMarkRealtimeRestorer rt(
simulated_event_loop_->runtime_realtime_priority() > 0);
DoCallCallback([monotonic_now]() { return monotonic_now; }, context);
+ simulated_event_loop_->ClearContext();
}
msgs_.pop_front();
@@ -1234,6 +1237,7 @@
ScopedMarkRealtimeRestorer rt(
simulated_event_loop_->runtime_realtime_priority() > 0);
Call([monotonic_now]() { return monotonic_now; }, monotonic_now);
+ simulated_event_loop_->ClearContext();
}
}
@@ -1283,6 +1287,7 @@
[this](monotonic_clock::time_point sleep_time) {
Schedule(sleep_time);
});
+ simulated_event_loop_->ClearContext();
}
}
diff --git a/frc971/control_loops/BUILD b/frc971/control_loops/BUILD
index aed829e..4491bba 100644
--- a/frc971/control_loops/BUILD
+++ b/frc971/control_loops/BUILD
@@ -17,6 +17,7 @@
"//aos:flatbuffers",
"//aos:json_to_flatbuffer",
"//aos/events:simulated_event_loop",
+ "//aos/network:testing_time_converter",
"//aos/testing:googletest",
"//aos/testing:test_logging",
"//aos/time",
diff --git a/frc971/control_loops/control_loop_test.h b/frc971/control_loops/control_loop_test.h
index 576d476..90c93c7 100644
--- a/frc971/control_loops/control_loop_test.h
+++ b/frc971/control_loops/control_loop_test.h
@@ -7,6 +7,7 @@
#include "aos/events/simulated_event_loop.h"
#include "aos/flatbuffers.h"
#include "aos/json_to_flatbuffer.h"
+#include "aos/network/testing_time_converter.h"
#include "aos/testing/test_logging.h"
#include "aos/time/time.h"
#include "frc971/input/joystick_state_generated.h"
@@ -27,16 +28,31 @@
public:
ControlLoopTestTemplated(
aos::FlatbufferDetachedBuffer<aos::Configuration> configuration,
- ::std::chrono::nanoseconds dt = kTimeTick)
+ ::std::chrono::nanoseconds dt = kTimeTick,
+ std::vector<std::vector<aos::logger::BootTimestamp>> times = {})
: configuration_(std::move(configuration)),
+ time_converter_(
+ aos::configuration::NodesCount(&configuration_.message())),
event_loop_factory_(&configuration_.message()),
- dt_(dt),
- robot_status_event_loop_(MakeEventLoop(
- "robot_status",
- aos::configuration::MultiNode(event_loop_factory_.configuration())
- ? aos::configuration::GetNode(
- event_loop_factory_.configuration(), "roborio")
- : nullptr)) {
+ dt_(dt) {
+ event_loop_factory()->SetTimeConverter(&time_converter_);
+
+ // We need to setup the time converter before any event loop has been
+ // created. Otherwise, the event loop will read the boot uuid and we'll be
+ // unable to change it.
+ if (times.empty()) {
+ time_converter_.StartEqual();
+ } else {
+ for (const std::vector<aos::logger::BootTimestamp> &time : times) {
+ time_converter_.AddMonotonic(time);
+ }
+ }
+ robot_status_event_loop_ = MakeEventLoop(
+ "robot_status",
+ aos::configuration::MultiNode(event_loop_factory_.configuration())
+ ? aos::configuration::GetNode(event_loop_factory_.configuration(),
+ "roborio")
+ : nullptr);
aos::testing::EnableTestLogging();
robot_state_sender_ =
robot_status_event_loop_->MakeSender<::aos::RobotState>("/aos");
@@ -112,6 +128,10 @@
return &event_loop_factory_;
}
+ aos::message_bridge::TestingTimeConverter *time_converter() {
+ return &time_converter_;
+ }
+
private:
// Sends out all of the required queue messages.
void SendJoystickState() {
@@ -163,6 +183,8 @@
aos::FlatbufferDetachedBuffer<aos::Configuration> configuration_;
+ aos::message_bridge::TestingTimeConverter time_converter_;
+
aos::SimulatedEventLoopFactory event_loop_factory_;
const ::std::chrono::nanoseconds dt_;
diff --git a/y2020/control_loops/drivetrain/localizer_test.cc b/y2020/control_loops/drivetrain/localizer_test.cc
index 91b5505..9e9e7dd 100644
--- a/y2020/control_loops/drivetrain/localizer_test.cc
+++ b/y2020/control_loops/drivetrain/localizer_test.cc
@@ -5,7 +5,6 @@
#include "aos/events/logging/log_writer.h"
#include "aos/network/message_bridge_server_generated.h"
#include "aos/network/team_number.h"
-#include "aos/network/testing_time_converter.h"
#include "frc971/control_loops/control_loop_test.h"
#include "frc971/control_loops/drivetrain/drivetrain.h"
#include "frc971/control_loops/drivetrain/drivetrain_test_lib.h"
@@ -107,8 +106,13 @@
: frc971::testing::ControlLoopTest(
aos::configuration::ReadConfig(
"y2020/control_loops/drivetrain/simulation_config.json"),
- GetTest2020DrivetrainConfig().dt),
- time_converter_(aos::configuration::NodesCount(configuration())),
+ GetTest2020DrivetrainConfig().dt,
+ {{BootTimestamp::epoch() + kPiTimeOffset,
+ BootTimestamp::epoch() + kPiTimeOffset,
+ BootTimestamp::epoch() + kPiTimeOffset,
+ BootTimestamp::epoch() + kPiTimeOffset,
+ BootTimestamp::epoch() + kPiTimeOffset,
+ BootTimestamp::epoch() + kPiTimeOffset, BootTimestamp::epoch()}}),
roborio_(aos::configuration::GetNode(configuration(), "roborio")),
pi1_(aos::configuration::GetNode(configuration(), "pi1")),
test_event_loop_(MakeEventLoop("test", roborio_)),
@@ -138,16 +142,8 @@
drivetrain_plant_event_loop_(MakeEventLoop("plant", roborio_)),
drivetrain_plant_(drivetrain_plant_event_loop_.get(), dt_config_),
last_frame_(monotonic_now()) {
- event_loop_factory()->SetTimeConverter(&time_converter_);
CHECK_EQ(aos::configuration::GetNodeIndex(configuration(), roborio_), 6);
CHECK_EQ(aos::configuration::GetNodeIndex(configuration(), pi1_), 1);
- time_converter_.AddMonotonic({BootTimestamp::epoch() + kPiTimeOffset,
- BootTimestamp::epoch() + kPiTimeOffset,
- BootTimestamp::epoch() + kPiTimeOffset,
- BootTimestamp::epoch() + kPiTimeOffset,
- BootTimestamp::epoch() + kPiTimeOffset,
- BootTimestamp::epoch() + kPiTimeOffset,
- BootTimestamp::epoch()});
set_team_id(frc971::control_loops::testing::kTeamNumber);
set_battery_voltage(12.0);
@@ -351,8 +347,6 @@
}
}
- aos::message_bridge::TestingTimeConverter time_converter_;
-
const aos::Node *const roborio_;
const aos::Node *const pi1_;
diff --git a/y2022/control_loops/drivetrain/BUILD b/y2022/control_loops/drivetrain/BUILD
index 3442a39..18855a6 100644
--- a/y2022/control_loops/drivetrain/BUILD
+++ b/y2022/control_loops/drivetrain/BUILD
@@ -122,7 +122,6 @@
"//aos/events:simulated_event_loop",
"//aos/events/logging:log_writer",
"//aos/network:team_number",
- "//aos/network:testing_time_converter",
"//frc971/control_loops:control_loop_test",
"//frc971/control_loops:team_number_test_environment",
"//frc971/control_loops/drivetrain:drivetrain_lib",