From 8305c04921a0489dbc733cc491e8c42ef5f52e45 Mon Sep 17 00:00:00 2001
From: Mauro Passerino <mpasserino@irobot.com>
Date: Fri, 18 Aug 2023 11:29:07 +0100
Subject: [PATCH 1/2] Add TimersManager::is_running() / Exit spin() before
 destroying stuff

---
 .../rclcpp/executors/events_executor.hpp      |  2 +-
 .../rclcpp/executors/timers_manager.hpp       |  6 ++++++
 .../src/rclcpp/executors/events_executor.cpp  | 20 +++++++++++++++++++
 .../src/rclcpp/executors/timers_manager.cpp   |  5 +++++
 4 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/rclcpp/include/rclcpp/executors/events_executor.hpp b/rclcpp/include/rclcpp/executors/events_executor.hpp
index a9a0082d71..9b341548c9 100644
--- a/rclcpp/include/rclcpp/executors/events_executor.hpp
+++ b/rclcpp/include/rclcpp/executors/events_executor.hpp
@@ -73,7 +73,7 @@ class EventsExecutor : public rclcpp::Executor
 
   /// Default destrcutor.
   RCLCPP_PUBLIC
-  virtual ~EventsExecutor() = default;
+  virtual ~EventsExecutor();
 
   /// Events executor implementation of spin.
   /**
diff --git a/rclcpp/include/rclcpp/executors/timers_manager.hpp b/rclcpp/include/rclcpp/executors/timers_manager.hpp
index 6e89873565..19a7a2402f 100644
--- a/rclcpp/include/rclcpp/executors/timers_manager.hpp
+++ b/rclcpp/include/rclcpp/executors/timers_manager.hpp
@@ -150,6 +150,12 @@ class TimersManager
    */
   std::chrono::nanoseconds get_head_timeout();
 
+  /**
+   * @brief Function to check if the timers thread is running
+   * @return true if timers thread is running
+   */
+  bool is_running();
+
 private:
   RCLCPP_DISABLE_COPY(TimersManager)
 
diff --git a/rclcpp/src/rclcpp/executors/events_executor.cpp b/rclcpp/src/rclcpp/executors/events_executor.cpp
index 278e418691..40ce738a2f 100644
--- a/rclcpp/src/rclcpp/executors/events_executor.cpp
+++ b/rclcpp/src/rclcpp/executors/events_executor.cpp
@@ -52,6 +52,26 @@ EventsExecutor::EventsExecutor(
   entities_collector_->add_waitable(executor_notifier_);
 }
 
+EventsExecutor::~EventsExecutor()
+{
+  // Set 'spinning' to false and get previous value
+  auto executor_was_spinning = spinning.exchange(false);
+
+  if (executor_was_spinning) {
+    // Trigger the shutdown guard condition.
+    // This way, the 'events_queue_' will wake up since a new event has arrived.
+    // Then since 'spinning' is now false, the spin loop will exit.
+    shutdown_guard_condition_->trigger();
+
+    // The timers manager thread is stopped at the end of spin().
+    // We have to wait for timers manager thread to exit, otherwise
+    // the 'timers_manager_' will be destroyed while still being used on spin().
+    while (timers_manager_->is_running()) {
+      std::this_thread::sleep_for(1ms);
+    }
+  }
+}
+
 void
 EventsExecutor::spin()
 {
diff --git a/rclcpp/src/rclcpp/executors/timers_manager.cpp b/rclcpp/src/rclcpp/executors/timers_manager.cpp
index 35d73174f4..a0bae63e67 100644
--- a/rclcpp/src/rclcpp/executors/timers_manager.cpp
+++ b/rclcpp/src/rclcpp/executors/timers_manager.cpp
@@ -284,3 +284,8 @@ void TimersManager::remove_timer(TimerPtr timer)
 
   timer->clear_on_reset_callback();
 }
+
+bool TimersManager::is_running()
+{
+  return running_;
+}

From 8383cc5fb43b39ae79b323631b8e202769f2515e Mon Sep 17 00:00:00 2001
From: Mauro Passerino <mpasserino@irobot.com>
Date: Fri, 18 Aug 2023 11:30:57 +0100
Subject: [PATCH 2/2] Unset gc's callback on ~EventsExecutorNotifyWaitable()

---
 .../rclcpp/executors/events_executor_notify_waitable.hpp   | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/rclcpp/include/rclcpp/executors/events_executor_notify_waitable.hpp b/rclcpp/include/rclcpp/executors/events_executor_notify_waitable.hpp
index 21b89ae692..adea505fd4 100644
--- a/rclcpp/include/rclcpp/executors/events_executor_notify_waitable.hpp
+++ b/rclcpp/include/rclcpp/executors/events_executor_notify_waitable.hpp
@@ -41,7 +41,12 @@ class EventsExecutorNotifyWaitable final : public EventWaitable
 
   // Destructor
   RCLCPP_PUBLIC
-  virtual ~EventsExecutorNotifyWaitable() = default;
+  virtual ~EventsExecutorNotifyWaitable()
+  {
+    for (auto & gc : notify_guard_conditions_) {
+      gc->set_on_trigger_callback(nullptr);
+    }
+  }
 
   // The function is a no-op, since we only care of waking up the executor
   RCLCPP_PUBLIC