commit 3058690ac148fb9dca5fa9d1edab432189bbbe96
author Austin Schuh <austin.linux@gmail.com> Tue Mar 30 22:54:08 2021 -0700
committer Austin Schuh <austin.linux@gmail.com> Tue Mar 30 22:54:08 2021 -0700
tree 3b24a6428e3af36a862854ef0107f8a8d260dc5b
parent aba3b1fd734b1ca08fff7031e308c5f4adf776ac

Break Logger recursion

There are some cases where folks are using the on_logged_period to
rotate and stop the logger.  When they stop the logger, that triggers
the on_logged_period callback to be called again, potentially ending in
an infintate recursion.

Fix that by blocking the callback when flushing at shutdown.

Change-Id: I442b4f5a9d29d996db3e450bd4f652e33ac7feda

aos/events/logging/log_writer.cc

diff --git a/aos/events/logging/log_writer.cc b/aos/events/logging/log_writer.cc
index f68bbf6..2780922 100644
--- a/aos/events/logging/log_writer.cc
+++ b/aos/events/logging/log_writer.cc
@@ -22,7 +22,7 @@
       configuration_(configuration),
       name_(network::GetHostname()),
       timer_handler_(event_loop_->AddTimer(
-          [this]() { DoLogData(event_loop_->monotonic_now()); })),
+          [this]() { DoLogData(event_loop_->monotonic_now(), true); })),
       server_statistics_fetcher_(
           configuration::MultiNode(event_loop_->configuration())
               ? event_loop_->MakeFetcher<message_bridge::ServerStatistics>(
@@ -264,7 +264,11 @@
   CHECK(log_namer_) << ": Not logging right now";
 
   if (end_time != aos::monotonic_clock::min_time) {
-    DoLogData(end_time);
+    // Folks like to use the on_logged_period_ callback to trigger stop and
+    // start events.  We can't have those then recurse and try to stop again.
+    // Rather than making everything reentrant, let's just instead block the
+    // callback here.
+    DoLogData(end_time, false);
   }
   timer_handler_->Disable();
 
@@ -778,7 +782,8 @@
   last_synchronized_time_ = t;
 }
 
-void Logger::DoLogData(const monotonic_clock::time_point end_time) {
+void Logger::DoLogData(const monotonic_clock::time_point end_time,
+                       bool run_on_logged) {
   // We want to guarantee that messages aren't out of order by more than
   // max_out_of_order_duration.  To do this, we need sync points.  Every write
   // cycle should be a sync point.
@@ -788,7 +793,9 @@
     // per iteration, even if it is small.
     LogUntil(std::min(last_synchronized_time_ + polling_period_, end_time));
 
-    on_logged_period_();
+    if (run_on_logged) {
+      on_logged_period_();
+    }
 
     // If we missed cycles, we could be pretty far behind.  Spin until we are
     // caught up.

aos/events/logging/log_writer.h

diff --git a/aos/events/logging/log_writer.h b/aos/events/logging/log_writer.h
index 992633a..675e113 100644
--- a/aos/events/logging/log_writer.h
+++ b/aos/events/logging/log_writer.h
@@ -243,7 +243,8 @@
       aos::monotonic_clock::time_point monotonic_start_time,
       aos::realtime_clock::time_point realtime_start_time);
 
-  void DoLogData(const monotonic_clock::time_point end_time);
+  void DoLogData(const monotonic_clock::time_point end_time,
+                 bool run_on_logged);
 
   void WriteMissingTimestamps();