16.0.5

Signed-off-by: Audrow Nash <audrow@intrinsic.ai>

rclcpp/CHANGELOG.rst

@@ -2,6 +2,18 @@
 Changelog for package rclcpp
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+16.0.5 (2023-07-17)
+-------------------
+* warning: comparison of integer expressions of different signedness (`#2219 <https://github.com/ros2/rclcpp/issues/2219>`_) (`#2223 <https://github.com/ros2/rclcpp/issues/2223>`_)
+* Trigger the intraprocess guard condition with data (`#2164 <https://github.com/ros2/rclcpp/issues/2164>`_) (`#2167 <https://github.com/ros2/rclcpp/issues/2167>`_)
+* Implement validity checks for rclcpp::Clock (`#2040 <https://github.com/ros2/rclcpp/issues/2040>`_) (`#2210 <https://github.com/ros2/rclcpp/issues/2210>`_)
+* Contributors: Tomoya Fujita, mergify[bot]
+
+16.0.4 (2023-04-25)
+-------------------
+* use allocator via init_options argument. (`#2129 <https://github.com/ros2/rclcpp/issues/2129>`_) (`#2131 <https://github.com/ros2/rclcpp/issues/2131>`_)
+* Contributors: mergify[bot]
+
 16.0.3 (2023-01-10)
 -------------------
 * Fix SharedFuture from async_send_request never becomes valid (`#2044 <https://github.com/ros2/rclcpp/issues/2044>`_) (`#2076 <https://github.com/ros2/rclcpp/issues/2076>`_)

rclcpp/include/rclcpp/clock.hpp

@@ -137,6 +137,51 @@ public:
     Duration rel_time,
     Context::SharedPtr context = contexts::get_global_default_context());
 
+  /**
+   * Check if the clock is started.
+   *
+   * A started clock is a clock that reflects non-zero time.
+   * Typically a clock will be unstarted if it is using RCL_ROS_TIME with ROS time and
+   * nothing has been published on the clock topic yet.
+   *
+   * \return true if clock is started
+   * \throws std::runtime_error if the clock is not rcl_clock_valid
+   */
+  RCLCPP_PUBLIC
+  bool
+  started();
+
+  /**
+   * Wait until clock to start.
+   *
+   * \rclcpp::Clock::started
+   * \param context the context to wait in
+   * \return true if clock was already started or became started
+   * \throws std::runtime_error if the context is invalid or clock is not rcl_clock_valid
+   */
+  RCLCPP_PUBLIC
+  bool
+  wait_until_started(Context::SharedPtr context = contexts::get_global_default_context());
+
+  /**
+   * Wait for clock to start, with timeout.
+   *
+   * The timeout is waited in steady time.
+   *
+   * \rclcpp::Clock::started
+   * \param timeout the maximum time to wait for.
+   * \param context the context to wait in.
+   * \param wait_tick_ns the time to wait between each iteration of the wait loop (in nanoseconds).
+   * \return true if clock was or became valid
+   * \throws std::runtime_error if the context is invalid or clock is not rcl_clock_valid
+   */
+  RCLCPP_PUBLIC
+  bool
+  wait_until_started(
+    const rclcpp::Duration & timeout,
+    Context::SharedPtr context = contexts::get_global_default_context(),
+    const rclcpp::Duration & wait_tick_ns = rclcpp::Duration(0, static_cast<uint32_t>(1e7)));
+
   /**
    * Returns the clock of the type `RCL_ROS_TIME` is active.
    *

rclcpp/include/rclcpp/experimental/subscription_intra_process.hpp

@@ -118,6 +118,13 @@ public:
         return nullptr;
       }
     }
+
+    if (this->buffer_->has_data()) {
+      // If there is data still to be processed, indicate to the
+      // executor or waitset by triggering the guard condition.
+      this->trigger_guard_condition();
+    }
+
     return std::static_pointer_cast<void>(
       std::make_shared<std::pair<ConstMessageSharedPtr, MessageUniquePtr>>(
         std::pair<ConstMessageSharedPtr, MessageUniquePtr>(

rclcpp/package.xml

@@ -2,7 +2,7 @@
 <?xml-model href="http://download.ros.org/schema/package_format2.xsd" schematypens="http://www.w3.org/2001/XMLSchema"?>
 <package format="2">
   <name>rclcpp</name>
-  <version>16.0.3</version>
+  <version>16.0.5</version>
   <description>The ROS client library in C++.</description>
   <maintainer email="ivanpauno@ekumenlabs.com">Ivan Paunovic</maintainer>
   <maintainer email="jacob@openrobotics.org">Jacob Perron</maintainer>

rclcpp/src/rclcpp/clock.cpp

@@ -182,6 +182,71 @@ Clock::sleep_for(Duration rel_time, Context::SharedPtr context)
   return sleep_until(now() + rel_time, context);
 }
 
+bool
+Clock::started()
+{
+  if (!rcl_clock_valid(get_clock_handle())) {
+    throw std::runtime_error("clock is not rcl_clock_valid");
+  }
+  return rcl_clock_time_started(get_clock_handle());
+}
+
+bool
+Clock::wait_until_started(Context::SharedPtr context)
+{
+  if (!context || !context->is_valid()) {
+    throw std::runtime_error("context cannot be slept with because it's invalid");
+  }
+  if (!rcl_clock_valid(get_clock_handle())) {
+    throw std::runtime_error("clock cannot be waited on as it is not rcl_clock_valid");
+  }
+
+  if (started()) {
+    return true;
+  } else {
+    // Wait until the first non-zero time
+    return sleep_until(rclcpp::Time(0, 1, get_clock_type()), context);
+  }
+}
+
+bool
+Clock::wait_until_started(
+  const Duration & timeout,
+  Context::SharedPtr context,
+  const Duration & wait_tick_ns)
+{
+  if (!context || !context->is_valid()) {
+    throw std::runtime_error("context cannot be slept with because it's invalid");
+  }
+  if (!rcl_clock_valid(get_clock_handle())) {
+    throw std::runtime_error("clock cannot be waited on as it is not rcl_clock_valid");
+  }
+
+  Clock timeout_clock = Clock(RCL_STEADY_TIME);
+  Time start = timeout_clock.now();
+
+  // Check if the clock has started every wait_tick_ns nanoseconds
+  // Context check checks for rclcpp::shutdown()
+  while (!started() && context->is_valid()) {
+    if (timeout < wait_tick_ns) {
+      timeout_clock.sleep_for(timeout);
+    } else {
+      Duration time_left = start + timeout - timeout_clock.now();
+      if (time_left > wait_tick_ns) {
+        timeout_clock.sleep_for(Duration(wait_tick_ns));
+      } else {
+        timeout_clock.sleep_for(time_left);
+      }
+    }
+
+    if (timeout_clock.now() - start > timeout) {
+      return started();
+    }
+  }
+  return started();
+}
+
+
 bool
 Clock::ros_time_is_active()
 {

rclcpp/src/rclcpp/context.cpp

@@ -217,7 +217,7 @@ Context::init(
     if (0u == count) {
       ret = rcl_logging_configure_with_output_handler(
         &rcl_context_->global_arguments,
-        rcl_init_options_get_allocator(init_options_.get_rcl_init_options()),
+        rcl_init_options_get_allocator(init_options.get_rcl_init_options()),
         rclcpp_logging_output_handler);
       if (RCL_RET_OK != ret) {
         rcl_context_.reset();

rclcpp/test/rclcpp/executors/test_executors.cpp

@@ -593,3 +593,106 @@ TEST(TestExecutors, testSpinUntilFutureCompleteNodePtr) {
 
   rclcpp::shutdown();
 }
+
+template<typename T>
+class TestIntraprocessExecutors : public ::testing::Test
+{
+public:
+  static void SetUpTestCase()
+  {
+    rclcpp::init(0, nullptr);
+  }
+
+  static void TearDownTestCase()
+  {
+    rclcpp::shutdown();
+  }
+
+  void SetUp()
+  {
+    const auto test_info = ::testing::UnitTest::GetInstance()->current_test_info();
+    std::stringstream test_name;
+    test_name << test_info->test_case_name() << "_" << test_info->name();
+    node = std::make_shared<rclcpp::Node>("node", test_name.str());
+
+    callback_count = 0u;
+
+    const std::string topic_name = std::string("topic_") + test_name.str();
+
+    rclcpp::PublisherOptions po;
+    po.use_intra_process_comm = rclcpp::IntraProcessSetting::Enable;
+    publisher = node->create_publisher<test_msgs::msg::Empty>(topic_name, rclcpp::QoS(1), po);
+
+    auto callback = [this](test_msgs::msg::Empty::ConstSharedPtr) {
+        this->callback_count.fetch_add(1u);
+      };
+
+    rclcpp::SubscriptionOptions so;
+    so.use_intra_process_comm = rclcpp::IntraProcessSetting::Enable;
+    subscription =
+      node->create_subscription<test_msgs::msg::Empty>(
+      topic_name, rclcpp::QoS(kNumMessages), std::move(callback), so);
+  }
+
+  void TearDown()
+  {
+    publisher.reset();
+    subscription.reset();
+    node.reset();
+  }
+
+  const size_t kNumMessages = 100;
+
+  rclcpp::Node::SharedPtr node;
+  rclcpp::Publisher<test_msgs::msg::Empty>::SharedPtr publisher;
+  rclcpp::Subscription<test_msgs::msg::Empty>::SharedPtr subscription;
+  std::atomic_size_t callback_count;
+};
+
+TYPED_TEST_SUITE(TestIntraprocessExecutors, ExecutorTypes, ExecutorTypeNames);
+
+TYPED_TEST(TestIntraprocessExecutors, testIntraprocessRetrigger) {
+  // This tests that executors will continue to service intraprocess subscriptions in the case
+  // that publishers aren't continuing to publish.
+  // This was previously broken in that intraprocess guard conditions were only triggered on
+  // publish and the test was added to prevent future regressions.
+  const size_t kNumMessages = 100;
+
+  using ExecutorType = TypeParam;
+  ExecutorType executor;
+  executor.add_node(this->node);
+
+  EXPECT_EQ(0u, this->callback_count.load());
+  this->publisher->publish(test_msgs::msg::Empty());
+
+  // Wait for up to 5 seconds for the first message to come available.
+  const std::chrono::milliseconds sleep_per_loop(10);
+  int loops = 0;
+  while (1u != this->callback_count.load() && loops < 500) {
+    rclcpp::sleep_for(sleep_per_loop);
+    executor.spin_some();
+    loops++;
+  }
+  EXPECT_EQ(1u, this->callback_count.load());
+
+  // reset counter
+  this->callback_count.store(0u);
+
+  for (size_t ii = 0; ii < kNumMessages; ++ii) {
+    this->publisher->publish(test_msgs::msg::Empty());
+  }
+
+  // Fire a timer every 10ms up to 5 seconds waiting for subscriptions to be read.
+  loops = 0;
+  auto timer = this->node->create_wall_timer(
+    std::chrono::milliseconds(10), [this, &executor, &loops, &kNumMessages]() {
+      loops++;
+      if (kNumMessages == this->callback_count.load() ||
+      loops == 500)
+      {
+        executor.cancel();
+      }
+    });
+  executor.spin();
+  EXPECT_EQ(kNumMessages, this->callback_count.load());
+}

rclcpp/test/rclcpp/test_time.cpp

@@ -19,6 +19,7 @@
 #include <limits>
 #include <memory>
 #include <string>
+#include <thread>
 
 #include "rcl/error_handling.h"
 #include "rcl/time.h"
@@ -848,3 +849,72 @@ TEST_F(TestClockSleep, sleep_for_basic_ros) {
   sleep_thread.join();
   EXPECT_TRUE(sleep_succeeded);
 }
+
+class TestClockStarted : public ::testing::Test
+{
+protected:
+  void SetUp()
+  {
+    rclcpp::init(0, nullptr);
+  }
+
+  void TearDown()
+  {
+    rclcpp::shutdown();
+  }
+};
+
+TEST_F(TestClockStarted, started) {
+  // rclcpp::Clock ros_clock(RCL_ROS_TIME);
+  // auto ros_clock_handle = ros_clock.get_clock_handle();
+  //
+  // // At this point, the ROS clock is reading system time since the ROS time override isn't on
+  // // So we expect it to be started (it's extremely unlikely that system time is at epoch start)
+  // EXPECT_TRUE(ros_clock.started());
+  // EXPECT_TRUE(ros_clock.wait_until_started());
+  // EXPECT_TRUE(ros_clock.wait_until_started(rclcpp::Duration(0, static_cast<uint32_t>(1e7))));
+  // EXPECT_EQ(RCL_RET_OK, rcl_enable_ros_time_override(ros_clock_handle));
+  // EXPECT_TRUE(ros_clock.ros_time_is_active());
+  // EXPECT_FALSE(ros_clock.started());
+  // EXPECT_EQ(RCL_RET_OK, rcl_set_ros_time_override(ros_clock_handle, 1));
+  // EXPECT_TRUE(ros_clock.started());
+  //
+  // rclcpp::Clock system_clock(RCL_SYSTEM_TIME);
+  // EXPECT_TRUE(system_clock.started());
+  // EXPECT_TRUE(system_clock.wait_until_started());
+  // EXPECT_TRUE(system_clock.wait_until_started(rclcpp::Duration(0, static_cast<uint32_t>(1e7))));
+  //
+  // rclcpp::Clock steady_clock(RCL_STEADY_TIME);
+  // EXPECT_TRUE(steady_clock.started());
+  // EXPECT_TRUE(steady_clock.wait_until_started());
+  // EXPECT_TRUE(steady_clock.wait_until_started(rclcpp::Duration(0, static_cast<uint32_t>(1e7))));
+  //
+  // rclcpp::Clock uninit_clock(RCL_CLOCK_UNINITIALIZED);
+  // RCLCPP_EXPECT_THROW_EQ(
+  //   uninit_clock.started(), std::runtime_error("clock is not rcl_clock_valid"));
+  // RCLCPP_EXPECT_THROW_EQ(
+  //   uninit_clock.wait_until_started(rclcpp::Duration(0, static_cast<uint32_t>(1e7))),
+  //   std::runtime_error("clock cannot be waited on as it is not rcl_clock_valid"));
+}
+
+TEST_F(TestClockStarted, started_timeout) {
+  rclcpp::Clock ros_clock(RCL_ROS_TIME);
+  auto ros_clock_handle = ros_clock.get_clock_handle();
+
+  EXPECT_EQ(RCL_RET_OK, rcl_enable_ros_time_override(ros_clock_handle));
+  EXPECT_TRUE(ros_clock.ros_time_is_active());
+
+  EXPECT_EQ(RCL_RET_OK, rcl_set_ros_time_override(ros_clock_handle, 0));
+
+  EXPECT_FALSE(ros_clock.started());
+  EXPECT_FALSE(ros_clock.wait_until_started(rclcpp::Duration(0, static_cast<uint32_t>(1e7))));
+
+  std::thread t([]() {
+      std::this_thread::sleep_for(std::chrono::seconds(1));
+      rclcpp::shutdown();
+    });
+
+  // Test rclcpp shutdown escape hatch (otherwise this waits indefinitely)
+  EXPECT_FALSE(ros_clock.wait_until_started());
+  t.join();
+}

rclcpp_action/CHANGELOG.rst

@@ -3,6 +3,16 @@ Changelog for package rclcpp_action
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 
+16.0.5 (2023-07-17)
+-------------------
+
+16.0.4 (2023-04-25)
+-------------------
+* Revert "Revert "extract the result response before the callback is issued. (`#2133 <https://github.com/ros2/rclcpp/issues/2133>`_)" (`#2148 <https://github.com/ros2/rclcpp/issues/2148>`_)" (`#2152 <https://github.com/ros2/rclcpp/issues/2152>`_)
+* Revert "extract the result response before the callback is issued. (`#2133 <https://github.com/ros2/rclcpp/issues/2133>`_)" (`#2148 <https://github.com/ros2/rclcpp/issues/2148>`_)
+* extract the result response before the callback is issued. (`#2133 <https://github.com/ros2/rclcpp/issues/2133>`_)
+* Contributors: Tomoya Fujita
+
 16.0.3 (2023-01-10)
 -------------------
 

rclcpp_action/package.xml

@@ -2,7 +2,7 @@
 <?xml-model href="http://download.ros.org/schema/package_format2.xsd" schematypens="http://www.w3.org/2001/XMLSchema"?>
 <package format="2">
   <name>rclcpp_action</name>
-  <version>16.0.3</version>
+  <version>16.0.5</version>
   <description>Adds action APIs for C++.</description>
   <maintainer email="ivanpauno@ekumenlabs.com">Ivan Paunovic</maintainer>
   <maintainer email="jacob@openrobotics.org">Jacob Perron</maintainer>

rclcpp_action/src/client.cpp

@@ -319,14 +319,18 @@ ClientBase::handle_result_response(
   const rmw_request_id_t & response_header,
   std::shared_ptr<void> response)
 {
-  std::lock_guard<std::mutex> guard(pimpl_->result_requests_mutex);
-  const int64_t & sequence_number = response_header.sequence_number;
-  if (pimpl_->pending_result_responses.count(sequence_number) == 0) {
-    RCLCPP_ERROR(pimpl_->logger, "unknown result response, ignoring...");
-    return;
+  ResponseCallback response_callback;
+  {
+    std::lock_guard<std::mutex> guard(pimpl_->result_requests_mutex);
+    const int64_t & sequence_number = response_header.sequence_number;
+    if (pimpl_->pending_result_responses.count(sequence_number) == 0) {
+      RCLCPP_ERROR(pimpl_->logger, "unknown result response, ignoring...");
+      return;
+    }
+    response_callback = std::move(pimpl_->pending_result_responses[sequence_number]);
+    pimpl_->pending_result_responses.erase(sequence_number);
   }
-  pimpl_->pending_result_responses[sequence_number](response);
-  pimpl_->pending_result_responses.erase(sequence_number);
+  response_callback(response);
 }
 
 void

rclcpp_components/CHANGELOG.rst

@@ -2,6 +2,12 @@
 Changelog for package rclcpp_components
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+16.0.5 (2023-07-17)
+-------------------
+
+16.0.4 (2023-04-25)
+-------------------
+
 16.0.3 (2023-01-10)
 -------------------
 

rclcpp_components/package.xml

@@ -2,7 +2,7 @@
 <?xml-model href="http://download.ros.org/schema/package_format2.xsd" schematypens="http://www.w3.org/2001/XMLSchema"?>
 <package format="2">
   <name>rclcpp_components</name>
-  <version>16.0.3</version>
+  <version>16.0.5</version>
   <description>Package containing tools for dynamically loadable components</description>
   <maintainer email="ivanpauno@ekumenlabs.com">Ivan Paunovic</maintainer>
   <maintainer email="jacob@openrobotics.org">Jacob Perron</maintainer>

rclcpp_lifecycle/CHANGELOG.rst

@@ -3,6 +3,18 @@ Changelog for package rclcpp_lifecycle
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 
+16.0.5 (2023-07-17)
+-------------------
+* Fix thread safety in LifecycleNode::get_current_state() for Humble (`#2183 <https://github.com/ros2/rclcpp/issues/2183>`_)
+  * add initially-failing test case
+  * apply changes to LifecycleNodeInterfaceImpl from `#1756 <https://github.com/ros2/rclcpp/issues/1756>`_
+  * add static member to State for managing state_handle\_ access
+  * allow parallel read access in MutexMap
+* Contributors: Joseph Schornak
+
+16.0.4 (2023-04-25)
+-------------------
+
 16.0.3 (2023-01-10)
 -------------------
 

rclcpp_lifecycle/CMakeLists.txt

@@ -20,6 +20,8 @@ find_package(lifecycle_msgs REQUIRED)
 add_library(rclcpp_lifecycle
   src/lifecycle_node.cpp
   src/managed_entity.cpp
+  src/mutex_map.cpp
+  src/mutex_map.hpp
   src/node_interfaces/lifecycle_node_interface.cpp
   src/state.cpp
   src/transition.cpp

rclcpp_lifecycle/include/rclcpp_lifecycle/state.hpp

@@ -24,6 +24,8 @@
 
 namespace rclcpp_lifecycle
 {
+/// Forward declaration of mutex helper class
+class MutexMap;
 
 /// Abstract class for the Lifecycle's states.
 /**
@@ -92,6 +94,21 @@ protected:
   bool owns_rcl_state_handle_;
 
   rcl_lifecycle_state_t * state_handle_;
+
+private:
+  /// Maps state handle mutexes to each instance of State.
+  /**
+   * \details A mutex is added to this map when each new instance of State is constructed.
+   *
+   * The mutex is removed when the instance of State is destroyed.
+   *
+   * The mutex is locked while state_handle_ is being accessed.
+   *
+   * This static member exists to allow implementing the fix described in ros2/rclcpp#1756
+   * in Humble without breaking ABI compatibility, since adding a new static data
+   * member is permitted under REP-0009.
+   */
+  static MutexMap state_handle_mutex_map_;
 };
 
 }  // namespace rclcpp_lifecycle

rclcpp_lifecycle/package.xml

@@ -2,7 +2,7 @@
 <?xml-model href="http://download.ros.org/schema/package_format2.xsd" schematypens="http://www.w3.org/2001/XMLSchema"?>
 <package format="2">
   <name>rclcpp_lifecycle</name>
-  <version>16.0.3</version>
+  <version>16.0.5</version>
   <description>Package containing a prototype for lifecycle implementation</description>
   <maintainer email="ivanpauno@ekumenlabs.com">Ivan Paunovic</maintainer>
   <maintainer email="jacob@openrobotics.org">Jacob Perron</maintainer>

rclcpp_lifecycle/src/lifecycle_node_interface_impl.hpp

@@ -20,6 +20,7 @@
 #include <functional>
 #include <map>
 #include <memory>
+#include <mutex>
 #include <string>
 #include <vector>
 #include <utility>
@@ -64,7 +65,11 @@ public:
   ~LifecycleNodeInterfaceImpl()
   {
     rcl_node_t * node_handle = node_base_interface_->get_rcl_node_handle();
-    auto ret = rcl_lifecycle_state_machine_fini(&state_machine_, node_handle);
+    rcl_ret_t ret;
+    {
+      std::lock_guard<std::recursive_mutex> lock(state_machine_mutex_);
+      ret = rcl_lifecycle_state_machine_fini(&state_machine_, node_handle);
+    }
     if (ret != RCL_RET_OK) {
       RCUTILS_LOG_FATAL_NAMED(
         "rclcpp_lifecycle",
@@ -78,7 +83,6 @@ public:
     rcl_node_t * node_handle = node_base_interface_->get_rcl_node_handle();
     const rcl_node_options_t * node_options =
       rcl_node_get_options(node_base_interface_->get_rcl_node_handle());
-    state_machine_ = rcl_lifecycle_get_zero_initialized_state_machine();
     auto state_machine_options = rcl_lifecycle_get_default_state_machine_options();
     state_machine_options.enable_com_interface = enable_communication_interface;
     state_machine_options.allocator = node_options->allocator;
@@ -89,6 +93,8 @@ public:
     // The publisher takes a C-Typesupport since the publishing (i.e. creating
     // the message) is done fully in RCL.
     // Services are handled in C++, so that it needs a C++ typesupport structure.
+    std::lock_guard<std::recursive_mutex> lock(state_machine_mutex_);
+    state_machine_ = rcl_lifecycle_get_zero_initialized_state_machine();
     rcl_ret_t ret = rcl_lifecycle_state_machine_init(
       &state_machine_,
       node_handle,
@@ -105,6 +111,8 @@ public:
               node_base_interface_->get_name());
     }
 
+    current_state_ = State(state_machine_.current_state);
+
     if (enable_communication_interface) {
       { // change_state
         auto cb = std::bind(
@@ -206,28 +214,30 @@ public:
     std::shared_ptr<ChangeStateSrv::Response> resp)
   {
     (void)header;
-    if (rcl_lifecycle_state_machine_is_initialized(&state_machine_) != RCL_RET_OK) {
-      throw std::runtime_error(
-              "Can't get state. State machine is not initialized.");
-    }
-
-    auto transition_id = req->transition.id;
-    // if there's a label attached to the request,
-    // we check the transition attached to this label.
-    // we further can't compare the id of the looked up transition
-    // because ros2 service call defaults all intergers to zero.
-    // that means if we call ros2 service call ... {transition: {label: shutdown}}
-    // the id of the request is 0 (zero) whereas the id from the lookup up transition
-    // can be different.
-    // the result of this is that the label takes presedence of the id.
-    if (req->transition.label.size() != 0) {
-      auto rcl_transition = rcl_lifecycle_get_transition_by_label(
-        state_machine_.current_state, req->transition.label.c_str());
-      if (rcl_transition == nullptr) {
-        resp->success = false;
-        return;
+    std::uint8_t transition_id;
+    {
+      std::lock_guard<std::recursive_mutex> lock(state_machine_mutex_);
+      if (rcl_lifecycle_state_machine_is_initialized(&state_machine_) != RCL_RET_OK) {
+        throw std::runtime_error("Can't get state. State machine is not initialized.");
+      }
+      transition_id = req->transition.id;
+      // if there's a label attached to the request,
+      // we check the transition attached to this label.
+      // we further can't compare the id of the looked up transition
+      // because ros2 service call defaults all intergers to zero.
+      // that means if we call ros2 service call ... {transition: {label: shutdown}}
+      // the id of the request is 0 (zero) whereas the id from the lookup up transition
+      // can be different.
+      // the result of this is that the label takes presedence of the id.
+      if (req->transition.label.size() != 0) {
+        auto rcl_transition = rcl_lifecycle_get_transition_by_label(
+          state_machine_.current_state, req->transition.label.c_str());
+        if (rcl_transition == nullptr) {
+          resp->success = false;
+          return;
+        }
+        transition_id = static_cast<std::uint8_t>(rcl_transition->id);
       }
-      transition_id = static_cast<std::uint8_t>(rcl_transition->id);
     }
 
     node_interfaces::LifecycleNodeInterface::CallbackReturn cb_return_code;
@@ -248,6 +258,7 @@ public:
   {
     (void)header;
     (void)req;
+    std::lock_guard<std::recursive_mutex> lock(state_machine_mutex_);
     if (rcl_lifecycle_state_machine_is_initialized(&state_machine_) != RCL_RET_OK) {
       throw std::runtime_error(
               "Can't get state. State machine is not initialized.");
@@ -264,6 +275,7 @@ public:
   {
     (void)header;
     (void)req;
+    std::lock_guard<std::recursive_mutex> lock(state_machine_mutex_);
     if (rcl_lifecycle_state_machine_is_initialized(&state_machine_) != RCL_RET_OK) {
       throw std::runtime_error(
               "Can't get available states. State machine is not initialized.");
@@ -286,6 +298,7 @@ public:
   {
     (void)header;
     (void)req;
+    std::lock_guard<std::recursive_mutex> lock(state_machine_mutex_);
     if (rcl_lifecycle_state_machine_is_initialized(&state_machine_) != RCL_RET_OK) {
       throw std::runtime_error(
               "Can't get available transitions. State machine is not initialized.");
@@ -313,6 +326,7 @@ public:
   {
     (void)header;
     (void)req;
+    std::lock_guard<std::recursive_mutex> lock(state_machine_mutex_);
     if (rcl_lifecycle_state_machine_is_initialized(&state_machine_) != RCL_RET_OK) {
       throw std::runtime_error(
               "Can't get available transitions. State machine is not initialized.");
@@ -343,6 +357,7 @@ public:
   get_available_states()
   {
     std::vector<State> states;
+    std::lock_guard<std::recursive_mutex> lock(state_machine_mutex_);
     states.reserve(state_machine_.transition_map.states_size);
 
     for (unsigned int i = 0; i < state_machine_.transition_map.states_size; ++i) {
@@ -355,6 +370,7 @@ public:
   get_available_transitions()
   {
     std::vector<Transition> transitions;
+    std::lock_guard<std::recursive_mutex> lock(state_machine_mutex_);
     transitions.reserve(state_machine_.current_state->valid_transition_size);
 
     for (unsigned int i = 0; i < state_machine_.current_state->valid_transition_size; ++i) {
@@ -367,6 +383,7 @@ public:
   get_transition_graph()
   {
     std::vector<Transition> transitions;
+    std::lock_guard<std::recursive_mutex> lock(state_machine_mutex_);
     transitions.reserve(state_machine_.transition_map.transitions_size);
 
     for (unsigned int i = 0; i < state_machine_.transition_map.transitions_size; ++i) {
@@ -378,26 +395,32 @@ public:
   rcl_ret_t
   change_state(std::uint8_t transition_id, LifecycleNodeInterface::CallbackReturn & cb_return_code)
   {
-    if (rcl_lifecycle_state_machine_is_initialized(&state_machine_) != RCL_RET_OK) {
-      RCUTILS_LOG_ERROR(
-        "Unable to change state for state machine for %s: %s",
-        node_base_interface_->get_name(), rcl_get_error_string().str);
-      return RCL_RET_ERROR;
-    }
-
     constexpr bool publish_update = true;
-    // keep the initial state to pass to a transition callback
-    State initial_state(state_machine_.current_state);
+    State initial_state;
+    unsigned int current_state_id;
 
-    if (
-      rcl_lifecycle_trigger_transition_by_id(
-        &state_machine_, transition_id, publish_update) != RCL_RET_OK)
     {
-      RCUTILS_LOG_ERROR(
-        "Unable to start transition %u from current state %s: %s",
-        transition_id, state_machine_.current_state->label, rcl_get_error_string().str);
-      rcutils_reset_error();
-      return RCL_RET_ERROR;
+      std::lock_guard<std::recursive_mutex> lock(state_machine_mutex_);
+      if (rcl_lifecycle_state_machine_is_initialized(&state_machine_) != RCL_RET_OK) {
+        RCUTILS_LOG_ERROR(
+          "Unable to change state for state machine for %s: %s",
+          node_base_interface_->get_name(), rcl_get_error_string().str);
+        return RCL_RET_ERROR;
+      }
+      // keep the initial state to pass to a transition callback
+      initial_state = State(state_machine_.current_state);
+
+      if (
+        rcl_lifecycle_trigger_transition_by_id(
+          &state_machine_, transition_id, publish_update) != RCL_RET_OK)
+      {
+        RCUTILS_LOG_ERROR(
+          "Unable to start transition %u from current state %s: %s",
+          transition_id, state_machine_.current_state->label, rcl_get_error_string().str);
+        rcutils_reset_error();
+        return RCL_RET_ERROR;
+      }
+      current_state_id = state_machine_.current_state->id;
     }
 
     auto get_label_for_return_code =
@@ -411,18 +434,22 @@ public:
         return rcl_lifecycle_transition_error_label;
       };
 
-    cb_return_code = execute_callback(state_machine_.current_state->id, initial_state);
+    cb_return_code = execute_callback(current_state_id, initial_state);
     auto transition_label = get_label_for_return_code(cb_return_code);
 
-    if (
-      rcl_lifecycle_trigger_transition_by_label(
-        &state_machine_, transition_label, publish_update) != RCL_RET_OK)
     {
-      RCUTILS_LOG_ERROR(
-        "Failed to finish transition %u. Current state is now: %s (%s)",
-        transition_id, state_machine_.current_state->label, rcl_get_error_string().str);
-      rcutils_reset_error();
-      return RCL_RET_ERROR;
+      std::lock_guard<std::recursive_mutex> lock(state_machine_mutex_);
+      if (
+        rcl_lifecycle_trigger_transition_by_label(
+          &state_machine_, transition_label, publish_update) != RCL_RET_OK)
+      {
+        RCUTILS_LOG_ERROR(
+          "Failed to finish transition %u. Current state is now: %s (%s)",
+          transition_id, state_machine_.current_state->label, rcl_get_error_string().str);
+        rcutils_reset_error();
+        return RCL_RET_ERROR;
+      }
+      current_state_id = state_machine_.current_state->id;
     }
 
     // error handling ?!
@@ -430,8 +457,9 @@ public:
     if (cb_return_code == node_interfaces::LifecycleNodeInterface::CallbackReturn::ERROR) {
       RCUTILS_LOG_WARN("Error occurred while doing error handling.");
 
-      auto error_cb_code = execute_callback(state_machine_.current_state->id, initial_state);
+      auto error_cb_code = execute_callback(current_state_id, initial_state);
       auto error_cb_label = get_label_for_return_code(error_cb_code);
+      std::lock_guard<std::recursive_mutex> lock(state_machine_mutex_);
       if (
         rcl_lifecycle_trigger_transition_by_label(
           &state_machine_, error_cb_label, publish_update) != RCL_RET_OK)
@@ -476,8 +504,13 @@ public:
   const State & trigger_transition(
     const char * transition_label, LifecycleNodeInterface::CallbackReturn & cb_return_code)
   {
-    auto transition =
-      rcl_lifecycle_get_transition_by_label(state_machine_.current_state, transition_label);
+    const rcl_lifecycle_transition_t * transition;
+    {
+      std::lock_guard<std::recursive_mutex> lock(state_machine_mutex_);
+
+      transition =
+        rcl_lifecycle_get_transition_by_label(state_machine_.current_state, transition_label);
+    }
     if (transition) {
       change_state(static_cast<uint8_t>(transition->id), cb_return_code);
     }
@@ -534,6 +567,7 @@ public:
     }
   }
 
+  mutable std::recursive_mutex state_machine_mutex_;
   rcl_lifecycle_state_machine_t state_machine_;
   State current_state_;
   std::map<

rclcpp_lifecycle/src/mutex_map.cpp

@@ -0,0 +1,43 @@
+// Copyright 2023 PickNik, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "mutex_map.hpp"
+
+#include <memory>
+#include <shared_mutex>
+#include <thread>
+
+namespace rclcpp_lifecycle
+{
+void MutexMap::add(const State * key)
+{
+  // Adding a new mutex to the map requires exclusive access
+  std::unique_lock lock(map_access_mutex_);
+  mutex_map_.emplace(key, std::make_unique<std::recursive_mutex>());
+}
+
+std::recursive_mutex & MutexMap::getMutex(const State * key) const
+{
+  // Multiple threads can retrieve mutexes from the map at the same time
+  std::shared_lock lock(map_access_mutex_);
+  return *(mutex_map_.at(key));
+}
+
+void MutexMap::remove(const State * key)
+{
+  // Removing a mutex from the map requires exclusive access
+  std::unique_lock lock(map_access_mutex_);
+  mutex_map_.erase(key);
+}
+}  // namespace rclcpp_lifecycle

rclcpp_lifecycle/src/mutex_map.hpp

@@ -0,0 +1,65 @@
+// Copyright 2023 PickNik, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef MUTEX_MAP_HPP_
+#define MUTEX_MAP_HPP_
+
+#include <map>
+#include <memory>
+#include <mutex>
+#include <shared_mutex>
+
+#include "rclcpp_lifecycle/state.hpp"
+
+namespace rclcpp_lifecycle
+{
+/// @brief Associates instances of recursive_mutex with instances of State.
+class MutexMap
+{
+public:
+  MutexMap() = default;
+
+  /// \brief Add a new mutex for an instance of State.
+  /**
+   * \param[in] key Raw pointer to the instance of State which will use the mutex.
+   */
+  void add(const State * key);
+
+  /// \brief Retrieve the mutex for an instance of State.
+  /**
+   * \param key Raw pointer to an instance of State.
+   * \return A reference to the mutex associated with the key.
+   */
+  std::recursive_mutex & getMutex(const State * key) const;
+
+  /// \brief Remove the mutex for an instance of State.
+  /**
+   * \param key Raw pointer to an instance of State.
+   */
+  void remove(const State * key);
+
+private:
+  /// \brief Map that stores the mutexes
+  /**
+   * \details The mutexes are emplaced as unique_ptrs because mutexes are
+   * not copyable or movable.
+   */
+  std::map<const State *, std::unique_ptr<std::recursive_mutex>> mutex_map_;
+
+  /// @brief Controls access to mutex_map_.
+  mutable std::shared_mutex map_access_mutex_;
+};
+}  // namespace rclcpp_lifecycle
+
+#endif  // MUTEX_MAP_HPP_

rclcpp_lifecycle/src/state.cpp

@@ -25,12 +25,17 @@
 
 #include "rcutils/allocator.h"
 
+#include "mutex_map.hpp"
+
 namespace rclcpp_lifecycle
 {
+MutexMap State::state_handle_mutex_map_;
 
 State::State(rcutils_allocator_t allocator)
 : State(lifecycle_msgs::msg::State::PRIMARY_STATE_UNKNOWN, "unknown", allocator)
-{}
+{
+  state_handle_mutex_map_.add(this);
+}
 
 State::State(
   uint8_t id,
@@ -40,6 +45,8 @@ State::State(
   owns_rcl_state_handle_(true),
   state_handle_(nullptr)
 {
+  state_handle_mutex_map_.add(this);
+
   if (label.empty()) {
     throw std::runtime_error("Lifecycle State cannot have an empty label.");
   }
@@ -67,6 +74,8 @@ State::State(
   owns_rcl_state_handle_(false),
   state_handle_(nullptr)
 {
+  state_handle_mutex_map_.add(this);
+
   if (!rcl_lifecycle_state_handle) {
     throw std::runtime_error("rcl_lifecycle_state_handle is null");
   }
@@ -78,12 +87,15 @@ State::State(const State & rhs)
   owns_rcl_state_handle_(false),
   state_handle_(nullptr)
 {
+  state_handle_mutex_map_.add(this);
+
   *this = rhs;
 }
 
 State::~State()
 {
   reset();
+  state_handle_mutex_map_.remove(this);
 }
 
 State &
@@ -93,6 +105,8 @@ State::operator=(const State & rhs)
     return *this;
   }
 
+  const auto lock = std::lock_guard<std::recursive_mutex>(state_handle_mutex_map_.getMutex(this));
+
   // reset all currently used resources
   reset();
 
@@ -128,6 +142,7 @@ State::operator=(const State & rhs)
 uint8_t
 State::id() const
 {
+  const auto lock = std::lock_guard<std::recursive_mutex>(state_handle_mutex_map_.getMutex(this));
   if (!state_handle_) {
     throw std::runtime_error("Error in state! Internal state_handle is NULL.");
   }
@@ -137,6 +152,7 @@ State::id() const
 std::string
 State::label() const
 {
+  const auto lock = std::lock_guard<std::recursive_mutex>(state_handle_mutex_map_.getMutex(this));
   if (!state_handle_) {
     throw std::runtime_error("Error in state! Internal state_handle is NULL.");
   }
@@ -146,6 +162,8 @@ State::label() const
 void
 State::reset() noexcept
 {
+  const auto lock = std::lock_guard<std::recursive_mutex>(state_handle_mutex_map_.getMutex(this));
+
   if (!owns_rcl_state_handle_) {
     state_handle_ = nullptr;
   }

rclcpp_lifecycle/test/test_lifecycle_node.cpp

@@ -377,6 +377,28 @@ TEST_F(TestDefaultStateMachine, call_transitions_without_code) {
   EXPECT_EQ(finalized.id(), State::PRIMARY_STATE_FINALIZED);
 }
 
+TEST_F(TestDefaultStateMachine, get_current_state_thread_safety) {
+  auto test_node = std::make_shared<EmptyLifecycleNode>("testnode");
+  test_node->trigger_transition(lifecycle_msgs::msg::Transition::TRANSITION_CONFIGURE);
+
+  const auto check_state_fn = [](std::shared_ptr<rclcpp_lifecycle::LifecycleNode> node)
+    {
+      std::size_t count = 0;
+      while (count < 100000) {
+        node->get_current_state().id();
+        count++;
+      }
+    };
+
+  // Call get_current_state() on the same node repeatedly from two different threads.
+  std::thread thread_object_1(check_state_fn, test_node);
+  std::thread thread_object_2(check_state_fn, test_node);
+
+  // Test has succeeded if both threads finish without exceptions.
+  thread_object_1.join();
+  thread_object_2.join();
+}
+
 TEST_F(TestDefaultStateMachine, good_mood) {
   auto test_node = std::make_shared<MoodyLifecycleNode<GoodMood>>("testnode");