From 34f9e13e72ea4528bd96f1d739bc7a8711cc923b Mon Sep 17 00:00:00 2001
From: Barry Xu <barry.xu@sony.com>
Date: Sat, 9 Nov 2024 01:32:55 +0800
Subject: [PATCH] Fix the race condition while calling rcl_shutdown (#1353)

* Fix the race condition while calling rcl_shutdown

Signed-off-by: Barry Xu <barry.xu@sony.com>

* Avoid calling rcl_shutdown() multiple times on the same context

Signed-off-by: Barry Xu <barry.xu@sony.com>

* Multiple calls to Context::shutdown will throw an exception

Signed-off-by: Barry Xu <barry.xu@sony.com>

* Update the name of an exception

Signed-off-by: Barry Xu <barry.xu@sony.com>

---------

Signed-off-by: Barry Xu <barry.xu@sony.com>
---
 rclpy/src/rclpy/context.cpp    | 21 +++++++++++----------
 rclpy/src/rclpy/context.hpp    |  1 +
 rclpy/src/rclpy/exceptions.hpp |  5 +++++
 3 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/rclpy/src/rclpy/context.cpp b/rclpy/src/rclpy/context.cpp
index 6d7219a79..e6b7166fc 100644
--- a/rclpy/src/rclpy/context.cpp
+++ b/rclpy/src/rclpy/context.cpp
@@ -150,17 +150,18 @@ Context::ok()
 void
 Context::shutdown()
 {
-  {
-    std::lock_guard<std::mutex> guard{g_contexts_mutex};
-    auto iter = std::find(g_contexts.begin(), g_contexts.end(), rcl_context_.get());
-    if (iter != g_contexts.end()) {
-      g_contexts.erase(iter);
-    }
+  std::lock_guard<std::mutex> guard{g_contexts_mutex};
+  if (already_shutdown_) {
+    throw ContextAlreadyShutdown("Context already shutdown.");
   }
-
-  rcl_ret_t ret = rcl_shutdown(rcl_context_.get());
-  if (RCL_RET_OK != ret) {
-    throw RCLError("failed to shutdown");
+  auto iter = std::find(g_contexts.begin(), g_contexts.end(), rcl_context_.get());
+  if (iter != g_contexts.end()) {
+    g_contexts.erase(iter);
+    rcl_ret_t ret = rcl_shutdown(rcl_context_.get());
+    if (RCL_RET_OK != ret) {
+      throw RCLError("failed to shutdown");
+    }
+    already_shutdown_ = true;
   }
 }
 
diff --git a/rclpy/src/rclpy/context.hpp b/rclpy/src/rclpy/context.hpp
index cdeb14122..30259e3e4 100644
--- a/rclpy/src/rclpy/context.hpp
+++ b/rclpy/src/rclpy/context.hpp
@@ -101,6 +101,7 @@ class Context : public Destroyable, public std::enable_shared_from_this<Context>
 
 private:
   std::shared_ptr<rcl_context_t> rcl_context_;
+  bool already_shutdown_{false};
 };
 
 /// Define a pybind11 wrapper for an rclpy::Context
diff --git a/rclpy/src/rclpy/exceptions.hpp b/rclpy/src/rclpy/exceptions.hpp
index 521146fd0..1f061c081 100644
--- a/rclpy/src/rclpy/exceptions.hpp
+++ b/rclpy/src/rclpy/exceptions.hpp
@@ -80,6 +80,11 @@ class InvalidHandle : public std::runtime_error
   using std::runtime_error::runtime_error;
 };
 
+class ContextAlreadyShutdown : public std::runtime_error
+{
+  using std::runtime_error::runtime_error;
+};
+
 }  // namespace rclpy
 
 #endif  // RCLPY__EXCEPTIONS_HPP_