fix: prevent double-shutdown in rclcpp::Context

rclcpp/include/rclcpp/context.hpp

@@ -15,6 +15,7 @@
 #ifndef RCLCPP__CONTEXT_HPP_
 #define RCLCPP__CONTEXT_HPP_
 
+#include <atomic>
 #include <condition_variable>
 #include <functional>
 #include <memory>
@@ -378,6 +379,7 @@ class Context : public std::enable_shared_from_this<Context>
   std::shared_ptr<rcl_context_t> rcl_context_;
   rclcpp::InitOptions init_options_;
   std::string shutdown_reason_;
+  std::atomic<bool> is_shutting_down_{false};
 
   // Keep shared ownership of the global logging mutex.
   std::shared_ptr<std::recursive_mutex> logging_mutex_;

rclcpp/src/rclcpp/context.cpp

@@ -201,6 +201,8 @@ Context::init(
     throw rclcpp::ContextAlreadyInitialized();
   }
   this->clean_up();
+  // allow shutdown() to be called again after re-initialization
+  is_shutting_down_.store(false);
   rcl_context_t * context = new rcl_context_t;
   if (!context) {
     throw std::runtime_error("failed to allocate memory for rcl context");
@@ -319,6 +321,12 @@ Context::shutdown_reason() const
 bool
 Context::shutdown(const std::string & reason)
 {
+  // Prevent double-shutdown: the signal handler thread and the main thread
+  // may both call shutdown() during Ctrl-C.  Use an atomic flag to ensure
+  // only the first call proceeds; subsequent calls return immediately.
+  if (is_shutting_down_.exchange(true)) {
+    return false;
+  }
   // prevent races
   std::lock_guard<std::recursive_mutex> init_lock(init_mutex_);
   // ensure validity

rclcpp/test/rclcpp/test_context.cpp

@@ -13,6 +13,10 @@
 // limitations under the License.
 #include <gtest/gtest.h>
 
+#include <atomic>
+#include <thread>
+#include <vector>
+
 #include "rclcpp/context.hpp"
 #include "rclcpp/rclcpp.hpp"
 
@@ -215,6 +219,75 @@ TEST(TestContext, check_on_shutdown_callback_order_after_del) {
   EXPECT_TRUE(result[0] == 1 && result[1] == 3 && result[2] == 4 && result[3] == 0);
 }
 
+// This test checks that a reentrant call to shutdown(), e.g. issued from a
+// pre_shutdown callback, returns false instead of running the entire shutdown
+// sequence again (which used to recurse into the pre_shutdown callbacks and
+// call rcl_shutdown() twice).
+TEST(TestContext, reentrant_shutdown_from_pre_shutdown_callback) {
+  auto context = std::make_shared<rclcpp::Context>();
+  context->init(0, nullptr);
+
+  size_t pre_shutdown_calls = 0;
+  size_t on_shutdown_calls = 0;
+  bool reentrant_shutdown_result = true;
+
+  context->add_pre_shutdown_callback(
+    [&context, &pre_shutdown_calls, &reentrant_shutdown_result]() {
+      pre_shutdown_calls++;
+      reentrant_shutdown_result = context->shutdown("reentrant shutdown");
+    });
+  context->add_on_shutdown_callback(
+    [&on_shutdown_calls]() {
+      on_shutdown_calls++;
+    });
+
+  EXPECT_TRUE(context->shutdown("for test"));
+  EXPECT_FALSE(reentrant_shutdown_result);
+  EXPECT_EQ(pre_shutdown_calls, 1u);
+  EXPECT_EQ(on_shutdown_calls, 1u);
+  EXPECT_FALSE(context->is_valid());
+
+  // shutdown() must work again after re-initialization
+  context->init(0, nullptr);
+  EXPECT_TRUE(context->is_valid());
+  EXPECT_TRUE(context->shutdown("for test again"));
+  EXPECT_FALSE(context->is_valid());
+}
+
+// This test checks that concurrent calls to shutdown() from multiple threads
+// result in exactly one thread running the shutdown sequence, and the other
+// threads returning false without error.
+TEST(TestContext, concurrent_shutdown) {
+  auto context = std::make_shared<rclcpp::Context>();
+  context->init(0, nullptr);
+
+  std::atomic<size_t> pre_shutdown_calls{0};
+  context->add_pre_shutdown_callback(
+    [&pre_shutdown_calls]() {
+      pre_shutdown_calls++;
+    });
+
+  constexpr size_t num_threads = 8;
+  std::atomic<size_t> success_count{0};
+  std::vector<std::thread> threads;
+  threads.reserve(num_threads);
+  for (size_t i = 0; i < num_threads; ++i) {
+    threads.emplace_back(
+      [&context, &success_count]() {
+        if (context->shutdown("concurrent shutdown")) {
+          success_count++;
+        }
+      });
+  }
+  for (auto & thread : threads) {
+    thread.join();
+  }
+
+  EXPECT_EQ(success_count, 1u);
+  EXPECT_EQ(pre_shutdown_calls, 1u);
+  EXPECT_FALSE(context->is_valid());
+}
+
 // This test checks that contexts will be properly destroyed when leaving a scope, after a
 // guard condition has been created.
 TEST(TestContext, check_context_destroyed) {