Fix crash when simulated event loop gets destroyed before OnRun called
We were saving a std::function for each OnRun callback inside the
EventScheduler, and continuing to call it regardless of if the EventLoop
which added it was still alive. This was triggering segfaults when
trying to access the destroyed event loop. Instead, save a pointer to
the event loop, and clean it up when the event loop is destroyed so this
works as the user would expect.
While we are here, and since the behavior needs to change as part of
this change, only let OnRun callbacks be scheduled before the event loop
has run any. Test this too.
Change-Id: I467a00c9f0981669e77fd03927ceeb149a79e6c4
Signed-off-by: James Kuszmaul <james.kuszmaul@bluerivertech.com>
diff --git a/y2020/control_loops/drivetrain/localizer_test.cc b/y2020/control_loops/drivetrain/localizer_test.cc
index ae92f97..ca25137 100644
--- a/y2020/control_loops/drivetrain/localizer_test.cc
+++ b/y2020/control_loops/drivetrain/localizer_test.cc
@@ -113,6 +113,16 @@
BootTimestamp::epoch() + kPiTimeOffset, BootTimestamp::epoch()}}),
roborio_(aos::configuration::GetNode(configuration(), "roborio")),
pi1_(aos::configuration::GetNode(configuration(), "pi1")),
+ drivetrain_event_loop_(MakeEventLoop("drivetrain", roborio_)),
+ dt_config_(GetTest2020DrivetrainConfig()),
+ pi1_event_loop_(MakeEventLoop("test", pi1_)),
+ camera_sender_(
+ pi1_event_loop_->MakeSender<ImageMatchResult>("/pi1/camera")),
+ localizer_(drivetrain_event_loop_.get(), dt_config_),
+ drivetrain_(dt_config_, drivetrain_event_loop_.get(), &localizer_),
+ drivetrain_plant_event_loop_(MakeEventLoop("plant", roborio_)),
+ drivetrain_plant_(drivetrain_plant_event_loop_.get(), dt_config_),
+ last_frame_(monotonic_now()),
test_event_loop_(MakeEventLoop("test", roborio_)),
drivetrain_goal_sender_(
test_event_loop_->MakeSender<Goal>("/drivetrain")),
@@ -129,17 +139,7 @@
"/superstructure")),
server_statistics_sender_(
test_event_loop_->MakeSender<aos::message_bridge::ServerStatistics>(
- "/aos")),
- drivetrain_event_loop_(MakeEventLoop("drivetrain", roborio_)),
- dt_config_(GetTest2020DrivetrainConfig()),
- pi1_event_loop_(MakeEventLoop("test", pi1_)),
- camera_sender_(
- pi1_event_loop_->MakeSender<ImageMatchResult>("/pi1/camera")),
- localizer_(drivetrain_event_loop_.get(), dt_config_),
- drivetrain_(dt_config_, drivetrain_event_loop_.get(), &localizer_),
- drivetrain_plant_event_loop_(MakeEventLoop("plant", roborio_)),
- drivetrain_plant_(drivetrain_plant_event_loop_.get(), dt_config_),
- last_frame_(monotonic_now()) {
+ "/aos")) {
CHECK_EQ(aos::configuration::GetNodeIndex(configuration(), roborio_), 6);
CHECK_EQ(aos::configuration::GetNodeIndex(configuration(), pi1_), 1);
set_team_id(frc971::control_loops::testing::kTeamNumber);
@@ -348,15 +348,6 @@
const aos::Node *const roborio_;
const aos::Node *const pi1_;
- std::unique_ptr<aos::EventLoop> test_event_loop_;
- aos::Sender<Goal> drivetrain_goal_sender_;
- aos::Fetcher<Goal> drivetrain_goal_fetcher_;
- aos::Fetcher<frc971::control_loops::drivetrain::Status>
- drivetrain_status_fetcher_;
- aos::Sender<LocalizerControl> localizer_control_sender_;
- aos::Sender<superstructure::Status> superstructure_status_sender_;
- aos::Sender<aos::message_bridge::ServerStatistics> server_statistics_sender_;
-
std::unique_ptr<aos::EventLoop> drivetrain_event_loop_;
const frc971::control_loops::drivetrain::DrivetrainConfig<double> dt_config_;
@@ -370,6 +361,15 @@
DrivetrainSimulation drivetrain_plant_;
monotonic_clock::time_point last_frame_;
+ std::unique_ptr<aos::EventLoop> test_event_loop_;
+ aos::Sender<Goal> drivetrain_goal_sender_;
+ aos::Fetcher<Goal> drivetrain_goal_fetcher_;
+ aos::Fetcher<frc971::control_loops::drivetrain::Status>
+ drivetrain_status_fetcher_;
+ aos::Sender<LocalizerControl> localizer_control_sender_;
+ aos::Sender<superstructure::Status> superstructure_status_sender_;
+ aos::Sender<aos::message_bridge::ServerStatistics> server_statistics_sender_;
+
// A queue of camera frames so that we can add a time delay to the data
// coming from the cameras.
std::queue<std::tuple<aos::monotonic_clock::time_point,
@@ -595,13 +595,14 @@
// Tests that we don't blow up if we stop getting updates for an extended period
// of time and fall behind on fetching fron the cameras.
TEST_F(LocalizedDrivetrainTest, FetchersHandleTimeGap) {
+ std::unique_ptr<aos::EventLoop> test = MakeEventLoop("test", roborio_);
+
set_enable_cameras(false);
set_send_delay(std::chrono::seconds(0));
event_loop_factory()->set_network_delay(std::chrono::nanoseconds(1));
- test_event_loop_
- ->AddTimer([this]() { drivetrain_plant_.set_send_messages(false); })
- ->Schedule(test_event_loop_->monotonic_now());
- test_event_loop_->AddPhasedLoop(
+ test->AddTimer([this]() { drivetrain_plant_.set_send_messages(false); })
+ ->Schedule(test->monotonic_now());
+ test->AddPhasedLoop(
[this](int) {
auto builder = camera_sender_.MakeBuilder();
ImageMatchResultT image;
@@ -609,12 +610,11 @@
aos::RawSender::Error::kOk);
},
std::chrono::milliseconds(40));
- test_event_loop_
- ->AddTimer([this]() {
+ test->AddTimer([this]() {
drivetrain_plant_.set_send_messages(true);
SimulateSensorReset();
})
- ->Schedule(test_event_loop_->monotonic_now() + std::chrono::seconds(10));
+ ->Schedule(test->monotonic_now() + std::chrono::seconds(10));
RunFor(chrono::seconds(20));
}
diff --git a/y2020/control_loops/superstructure/superstructure_lib_test.cc b/y2020/control_loops/superstructure/superstructure_lib_test.cc
index 950593e..fad888c 100644
--- a/y2020/control_loops/superstructure/superstructure_lib_test.cc
+++ b/y2020/control_loops/superstructure/superstructure_lib_test.cc
@@ -554,9 +554,10 @@
expected_voltage);
}
- void StartSendingFinisherGoals() {
+ void RegisterFinisherGoalsLoop() {
test_event_loop_->AddPhasedLoop(
[this](int) {
+ if (!send_finisher_goals_) return;
auto builder = superstructure_goal_sender_.MakeBuilder();
auto shooter_goal_builder = builder.MakeBuilder<ShooterGoal>();
shooter_goal_builder.add_velocity_finisher(finisher_goal_);
@@ -569,6 +570,8 @@
dt());
}
+ void StartSendingFinisherGoals() { send_finisher_goals_ = true; }
+
// Sets the finisher velocity goal (radians/s)
void SetFinisherGoalAfter(const double velocity_finisher,
const monotonic_clock::duration time_offset) {
@@ -619,6 +622,8 @@
double finisher_goal_ = 0;
bool ball_in_finisher_ = false;
+
+ bool send_finisher_goals_ = false;
};
// Tests that the superstructure does nothing when the goal is to remain
@@ -1032,18 +1037,18 @@
// Tests that we detect that a ball was shot whenever the average angular
// velocity is lower than a certain threshold compared to the goal.
TEST_F(SuperstructureTest, BallsShot) {
- SetEnabled(true);
- WaitUntilZeroed();
+ bool run_test = false;
int balls_shot = 0;
// When there is a ball in the flywheel, the finisher velocity should drop
// below the goal
bool finisher_velocity_below_goal = false;
- StartSendingFinisherGoals();
-
test_event_loop_->AddPhasedLoop(
[&](int) {
+ if (!run_test) {
+ return;
+ }
ASSERT_TRUE(superstructure_status_fetcher_.Fetch());
const double finisher_velocity =
superstructure_status_fetcher_->shooter()
@@ -1078,6 +1083,13 @@
}
},
dt());
+ RegisterFinisherGoalsLoop();
+
+ SetEnabled(true);
+
+ WaitUntilZeroed();
+
+ StartSendingFinisherGoals();
constexpr int kFastShootingSpeed = 500;
constexpr int kSlowShootingSpeed = 300;