commit a0c41ba70d0d3f549d54372c49cdd6569f8be9f8
author Austin Schuh <austin.linux@gmail.com> Thu Sep 10 22:59:14 2020 -0700
committer Brian Silverman <bsilver16384@gmail.com> Wed Sep 16 21:10:24 2020 -0700
tree 2a4a346a81bb9f67ca0daec6f8cc93cb87cb0725
parent c6d6ee8d6980e4f518e424b9e36d1f065f90e626

Actually manage memory in the old-style AOS logging

LeakSanitizer should be happy with it now. It's also still just as
thread-safe.

Change-Id: Id09a0349657cf4f719267b053f0ea3d8ec366256

aos/events/aos_logging.cc

diff --git a/aos/events/aos_logging.cc b/aos/events/aos_logging.cc
index d312925..5b6f72c 100644
--- a/aos/events/aos_logging.cc
+++ b/aos/events/aos_logging.cc
@@ -5,7 +5,7 @@
 void AosLogToFbs::Initialize(Sender<logging::LogMessageFbs> log_sender) {
   log_sender_ = std::move(log_sender);
   if (log_sender_) {
-    logging::RegisterCallbackImplementation(
+    implementation_ = std::make_shared<logging::CallbackLogImplementation>(
         [this](const logging::LogMessage &message_data) {
           aos::Sender<logging::LogMessageFbs>::Builder message =
               log_sender_.MakeBuilder();
@@ -23,8 +23,7 @@
           builder.add_name(name_str);
 
           message.Send(builder.Finish());
-        },
-        false);
+        });
   }
 }
 

aos/events/aos_logging.h

diff --git a/aos/events/aos_logging.h b/aos/events/aos_logging.h
index b50569c..e99edb7 100644
--- a/aos/events/aos_logging.h
+++ b/aos/events/aos_logging.h
@@ -11,13 +11,14 @@
  public:
   AosLogToFbs() {}
 
-  // TODO(Tyler): Deregister logger on destruction to avoid memory leaks
-
   void Initialize(Sender<logging::LogMessageFbs> log_sender);
+  std::shared_ptr<logging::LogImplementation> implementation() const {
+    return implementation_;
+  }
 
  private:
   Sender<logging::LogMessageFbs> log_sender_;
-  logging::ScopedLogRestorer prev_logger_;
+  std::shared_ptr<logging::LogImplementation> implementation_;
 };
 
 }  // namespace aos

aos/events/event_loop.h

diff --git a/aos/events/event_loop.h b/aos/events/event_loop.h
index de397e2..1e7d25b 100644
--- a/aos/events/event_loop.h
+++ b/aos/events/event_loop.h
@@ -358,7 +358,7 @@
   // Returns the name of the underlying queue.
   const Channel *channel() const { return sender_->channel(); }
 
-  operator bool() { return sender_ ? true : false; }
+  operator bool() const { return sender_ ? true : false; }
 
   // Returns the time_points that the last message was sent at.
   aos::monotonic_clock::time_point monotonic_sent_time() const {

aos/events/shm_event_loop.cc

diff --git a/aos/events/shm_event_loop.cc b/aos/events/shm_event_loop.cc
index 29f01a4..406325d 100644
--- a/aos/events/shm_event_loop.cc
+++ b/aos/events/shm_event_loop.cc
@@ -964,9 +964,11 @@
   ReserveEvents();
 
   {
+    logging::ScopedLogRestorer prev_logger;
     AosLogToFbs aos_logger;
     if (!skip_logger_) {
       aos_logger.Initialize(MakeSender<logging::LogMessageFbs>("/aos"));
+      prev_logger.Swap(aos_logger.implementation());
     }
 
     aos::SetCurrentThreadName(name_.substr(0, 16));

aos/events/simulated_event_loop.cc

diff --git a/aos/events/simulated_event_loop.cc b/aos/events/simulated_event_loop.cc
index a7301bf..c429eef 100644
--- a/aos/events/simulated_event_loop.cc
+++ b/aos/events/simulated_event_loop.cc
@@ -570,9 +570,8 @@
   void Setup() {
     MaybeScheduleTimingReports();
     if (!skip_logger_) {
-      logging::ScopedLogRestorer prev_logger;
       log_sender_.Initialize(MakeSender<logging::LogMessageFbs>("/aos"));
-      log_impl_ = logging::GetImplementation();
+      log_impl_ = log_sender_.implementation();
     }
   }
 
@@ -614,7 +613,7 @@
   const pid_t tid_;
 
   AosLogToFbs log_sender_;
-  logging::LogImplementation *log_impl_ = nullptr;
+  std::shared_ptr<logging::LogImplementation> log_impl_ = nullptr;
 };
 
 void SimulatedEventLoopFactory::set_send_delay(
@@ -720,8 +719,8 @@
   const monotonic_clock::time_point monotonic_now =
       simulated_event_loop_->monotonic_now();
   logging::ScopedLogRestorer prev_logger;
-  if (simulated_event_loop_->log_impl_ != nullptr) {
-    logging::SetImplementation(simulated_event_loop_->log_impl_);
+  if (simulated_event_loop_->log_impl_) {
+    prev_logger.Swap(simulated_event_loop_->log_impl_);
   }
   Context context = msgs_.front()->context;
 
@@ -837,8 +836,8 @@
   const ::aos::monotonic_clock::time_point monotonic_now =
       simulated_event_loop_->monotonic_now();
   logging::ScopedLogRestorer prev_logger;
-  if (simulated_event_loop_->log_impl_ != nullptr) {
-    logging::SetImplementation(simulated_event_loop_->log_impl_);
+  if (simulated_event_loop_->log_impl_) {
+    prev_logger.Swap(simulated_event_loop_->log_impl_);
   }
   if (token_ != scheduler_->InvalidToken()) {
     scheduler_->Deschedule(token_);
@@ -889,8 +888,8 @@
   monotonic_clock::time_point monotonic_now =
       simulated_event_loop_->monotonic_now();
   logging::ScopedLogRestorer prev_logger;
-  if (simulated_event_loop_->log_impl_ != nullptr) {
-    logging::SetImplementation(simulated_event_loop_->log_impl_);
+  if (simulated_event_loop_->log_impl_) {
+    prev_logger.Swap(simulated_event_loop_->log_impl_);
   }
   Call(
       [monotonic_now]() { return monotonic_now; },