Fix crash in ~Executor for executors using the minimal constructor

rclcpp/include/rclcpp/executor.hpp

@@ -414,6 +414,7 @@ class Executor
   * that explicitly does not want to use the default implementation provided
   * by this class.
   */
+  RCLCPP_PUBLIC
   explicit Executor(const std::shared_ptr<rclcpp::Context> & context);
 
   /// Add a node to executor, execute the next available unit of work, and remove the node.

rclcpp/src/rclcpp/executor.cpp

@@ -97,8 +97,12 @@ Executor::~Executor()
 {
   std::lock_guard<std::mutex> guard(mutex_);
 
-  notify_waitable_->remove_guard_condition(interrupt_guard_condition_);
-  notify_waitable_->remove_guard_condition(shutdown_guard_condition_);
+  // notify_waitable_ is null when a derived executor used the constructor that
+  // does not initialize the base functionality.
+  if (notify_waitable_) {
+    notify_waitable_->remove_guard_condition(interrupt_guard_condition_);
+    notify_waitable_->remove_guard_condition(shutdown_guard_condition_);
+  }
   current_collection_.timers.update(
     {}, {},
     [this](auto timer) {wait_set_.remove_timer(std::move(timer));});
@@ -125,8 +129,8 @@ Executor::~Executor()
     {}, {},
     [this](auto waitable) {wait_set_.remove_waitable(std::move(waitable));});
 
-  // Remove shutdown callback handle registered to Context
-  if (!context_->remove_on_shutdown_callback(shutdown_callback_handle_)) {
+  // Remove shutdown callback handle registered to Context, if one was registered
+  if (notify_waitable_ && !context_->remove_on_shutdown_callback(shutdown_callback_handle_)) {
     RCUTILS_LOG_ERROR_NAMED(
       "rclcpp",
       "failed to remove registered on_shutdown callback");

rclcpp/test/rclcpp/test_executor.cpp

@@ -27,6 +27,7 @@
 
 #include "rclcpp/callback_group.hpp"
 #include "rclcpp/client.hpp"
+#include "rclcpp/contexts/default_context.hpp"
 #include "rclcpp/create_subscription.hpp"
 #include "rclcpp/exceptions.hpp"
 #include "rclcpp/executor.hpp"
@@ -64,6 +65,21 @@ class DummyExecutor : public rclcpp::Executor
   }
 };
 
+// Derived executor that uses the constructor which does not initialize the
+// base class functionality.
+class MinimalExecutor : public rclcpp::Executor
+{
+public:
+  MinimalExecutor()
+  : rclcpp::Executor(rclcpp::contexts::get_global_default_context())
+  {
+  }
+
+  void spin() override
+  {
+  }
+};
+
 class TestExecutor : public ::testing::Test
 {
 public:
@@ -117,6 +133,14 @@ TEST_F(TestExecutor, add_remove_node_thread_safe) {
   future.get();
 }
 
+TEST_F(TestExecutor, destroy_executor_without_base_functionality) {
+  EXPECT_NO_THROW(
+  {
+    auto executor = std::make_unique<MinimalExecutor>();
+    executor.reset();
+  });
+}
+
 TEST_F(TestExecutor, constructor_bad_guard_condition_init) {
   auto mock = mocking_utils::patch_and_return(
     "lib:rclcpp", rcl_guard_condition_init, RCL_RET_ERROR);