Fix recursion of AllowApplicationCreationDuring in LogReader
Change-Id: I051e7e687b6c06c9bc04bff35ec5ec9db9ba6bc1
Signed-off-by: James Kuszmaul <james.kuszmaul@bluerivertech.com>
diff --git a/aos/events/event_scheduler.cc b/aos/events/event_scheduler.cc
index 115b2c7..ae50917 100644
--- a/aos/events/event_scheduler.cc
+++ b/aos/events/event_scheduler.cc
@@ -459,6 +459,10 @@
}
void EventSchedulerScheduler::TemporarilyStopAndRun(std::function<void()> fn) {
+ if (in_on_run_) {
+ LOG(FATAL)
+ << "Can't call AllowApplicationCreationDuring from an OnRun callback.";
+ }
const bool was_running = is_running_;
if (is_running_) {
is_running_ = false;
@@ -475,11 +479,13 @@
for (EventScheduler *scheduler : schedulers_) {
scheduler->MaybeRunOnStartup();
}
+ in_on_run_ = true;
// We must trigger all the OnRun's *after* all the OnStartup callbacks are
// triggered because that is the contract that we have stated.
for (EventScheduler *scheduler : schedulers_) {
scheduler->MaybeRunOnRun();
}
+ in_on_run_ = false;
}
} // namespace aos
diff --git a/aos/events/event_scheduler.h b/aos/events/event_scheduler.h
index 848a1cf..55fa9a4 100644
--- a/aos/events/event_scheduler.h
+++ b/aos/events/event_scheduler.h
@@ -357,6 +357,8 @@
// True if we are running.
bool is_running_ = false;
+
+ bool in_on_run_ = false;
// The current time.
distributed_clock::time_point now_ = distributed_clock::epoch();
// List of schedulers to run in sync.
diff --git a/aos/events/logging/log_reader.h b/aos/events/logging/log_reader.h
index bb3588a..d36db70 100644
--- a/aos/events/logging/log_reader.h
+++ b/aos/events/logging/log_reader.h
@@ -517,9 +517,18 @@
const monotonic_clock::time_point start_time =
monotonic_start_time(boot_count());
if (start_time == monotonic_clock::min_time) {
- LOG(ERROR)
- << "No start time, skipping, please figure out when this happens";
- NotifyLogfileStart();
+ if (event_loop_->node()) {
+ LOG(ERROR) << "No start time for "
+ << event_loop_->node()->name()->string_view()
+ << ", skipping.";
+ } else {
+ LOG(ERROR) << "No start time, skipping.";
+ }
+
+ // This is called from OnRun. There is too much complexity in supporting
+ // OnStartup callbacks from inside OnRun. Instead, schedule a timer for
+ // "now", and have that do what we need.
+ startup_timer_->Schedule(event_loop_->monotonic_now());
return;
}
if (node_event_loop_factory_) {
diff --git a/aos/events/simulated_event_loop_test.cc b/aos/events/simulated_event_loop_test.cc
index 0afa22e..23badae 100644
--- a/aos/events/simulated_event_loop_test.cc
+++ b/aos/events/simulated_event_loop_test.cc
@@ -2101,6 +2101,18 @@
"All ExitHandles must be destroyed before the factory");
}
+// Test that AllowApplicationCreationDuring can't happen in OnRun callbacks.
+TEST(SimulatedEventLoopDeathTest, AllowApplicationCreationDuringInOnRun) {
+ aos::FlatbufferDetachedBuffer<aos::Configuration> config =
+ aos::configuration::ReadConfig(
+ ArtifactPath("aos/events/multinode_pingpong_test_split_config.json"));
+ auto factory = std::make_unique<SimulatedEventLoopFactory>(&config.message());
+ NodeEventLoopFactory *pi1 = factory->GetNodeEventLoopFactory("pi1");
+ std::unique_ptr<EventLoop> loop = pi1->MakeEventLoop("foo");
+ loop->OnRun([&]() { factory->AllowApplicationCreationDuring([]() {}); });
+ EXPECT_DEATH(factory->RunFor(chrono::seconds(1)), "OnRun");
+}
+
// Tests that messages don't survive a reboot of a node.
TEST(SimulatedEventLoopTest, ChannelClearedOnReboot) {
aos::FlatbufferDetachedBuffer<aos::Configuration> config =
diff --git a/aos/network/timestamp_filter.cc b/aos/network/timestamp_filter.cc
index 77d3726..4a29dfc 100644
--- a/aos/network/timestamp_filter.cc
+++ b/aos/network/timestamp_filter.cc
@@ -1279,7 +1279,8 @@
<< " seconds after the last sample at " << std::get<0>(timestamps_[0])
<< ". Increase --time_estimation_buffer_seconds to greater than "
<< chrono::duration<double>(monotonic_now - std::get<0>(timestamps_[0]))
- .count();
+ .count()
+ << ", or set --force_timestamp_loading";
return;
}
CHECK_GT(monotonic_now, frozen_time_)
@@ -1287,7 +1288,8 @@
<< " before the frozen time of " << frozen_time_
<< ". Increase "
"--time_estimation_buffer_seconds to greater than "
- << chrono::duration<double>(frozen_time_ - monotonic_now).count();
+ << chrono::duration<double>(frozen_time_ - monotonic_now).count()
+ << ", or set --force_timestamp_loading";
// Future samples get quite a bit harder. We want the line to track the
// highest point without volating the slope constraint.
@@ -1333,7 +1335,8 @@
<< " seconds after the last sample at " << std::get<0>(timestamps_[0])
<< ". Increase --time_estimation_buffer_seconds to greater than "
<< chrono::duration<double>(monotonic_now - std::get<0>(timestamps_[0]))
- .count();
+ .count()
+ << ", or set --force_timestamp_loading";
// Back propagate the max velocity and remove any elements violating the
// velocity constraint. This is to handle the case where the offsets were
@@ -1357,8 +1360,8 @@
<< chrono::duration<double>(monotonic_now - std::get<0>(back)).count()
<< " seconds in the past. Increase --time_estimation_buffer_seconds "
"to greater than "
- << chrono::duration<double>(monotonic_now - std::get<0>(back))
- .count();
+ << chrono::duration<double>(monotonic_now - std::get<0>(back)).count()
+ << ", or set --force_timestamp_loading";
VLOG(1) << node_names_
<< " Removing now invalid sample during back propegation of "
<< TimeString(back);
@@ -1554,7 +1557,8 @@
<< ": " << node_names_
<< " Can't pop an already frozen sample. Increase "
"--time_estimation_buffer_seconds to greater than "
- << chrono::duration<double>(prior_dt).count();
+ << chrono::duration<double>(prior_dt).count()
+ << ", or set --force_timestamp_loading";
VLOG(1) << "Prior slope is too positive, removing prior point "
<< TimeString(*prior_it);