From c85d28eb723ff958d6cd614ccbc22aeafc3dc744 Mon Sep 17 00:00:00 2001 From: Ivo Ivanov Date: Wed, 24 Jun 2026 21:38:43 +0200 Subject: [PATCH 1/4] Fixed race condition on the RNG in rclcpp_action Signed-off-by: Ivo Ivanov --- rclcpp_action/src/client_base.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rclcpp_action/src/client_base.cpp b/rclcpp_action/src/client_base.cpp index 8f9d16b922..a823b65e92 100644 --- a/rclcpp_action/src/client_base.cpp +++ b/rclcpp_action/src/client_base.cpp @@ -181,6 +181,7 @@ class ClientBaseImpl std::map pending_cancel_responses; std::recursive_mutex cancel_requests_mutex; + std::mutex goal_id_rng_mutex; std::independent_bits_engine< std::mt19937, 8, unsigned int> random_bytes_generator; }; @@ -480,6 +481,7 @@ GoalUUID ClientBase::generate_goal_id() { GoalUUID goal_id; + std::lock_guard lock(pimpl_->goal_id_rng_mutex); // TODO(hidmic): Do something better than this for UUID generation. // std::generate( // goal_id.uuid.begin(), goal_id.uuid.end(), From 4ed254c64027bd8fc39e37d683844a2ee9655d1e Mon Sep 17 00:00:00 2001 From: Ivo Ivanov Date: Wed, 24 Jun 2026 21:39:27 +0200 Subject: [PATCH 2/4] The mutex should be reentrant Signed-off-by: Ivo Ivanov --- rclcpp_action/src/client_base.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rclcpp_action/src/client_base.cpp b/rclcpp_action/src/client_base.cpp index a823b65e92..07e8f70f4f 100644 --- a/rclcpp_action/src/client_base.cpp +++ b/rclcpp_action/src/client_base.cpp @@ -181,7 +181,7 @@ class ClientBaseImpl std::map pending_cancel_responses; std::recursive_mutex cancel_requests_mutex; - std::mutex goal_id_rng_mutex; + std::recursive_mutex goal_id_rng_mutex; std::independent_bits_engine< std::mt19937, 8, unsigned int> random_bytes_generator; }; From 57be7db75bc762e1ba08a84731236a897e0964e2 Mon Sep 17 00:00:00 2001 From: Ivo Ivanov Date: Thu, 25 Jun 2026 12:41:26 +0200 Subject: [PATCH 3/4] Fixed compile error, the lock guard parameter needs to be of the same type as the mutex type of course Signed-off-by: Ivo Ivanov --- rclcpp_action/src/client_base.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rclcpp_action/src/client_base.cpp b/rclcpp_action/src/client_base.cpp index 07e8f70f4f..03bc5c2d7a 100644 --- a/rclcpp_action/src/client_base.cpp +++ b/rclcpp_action/src/client_base.cpp @@ -481,7 +481,7 @@ GoalUUID ClientBase::generate_goal_id() { GoalUUID goal_id; - std::lock_guard lock(pimpl_->goal_id_rng_mutex); + std::lock_guard lock(pimpl_->goal_id_rng_mutex); // TODO(hidmic): Do something better than this for UUID generation. // std::generate( // goal_id.uuid.begin(), goal_id.uuid.end(), From 547b44e6362140b4faddf1d4cf5d1d1196425f6c Mon Sep 17 00:00:00 2001 From: Ivo Ivanov Date: Thu, 25 Jun 2026 15:13:11 +0200 Subject: [PATCH 4/4] Changed mutex back to be non-reentrant Signed-off-by: Ivo Ivanov --- rclcpp_action/src/client_base.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rclcpp_action/src/client_base.cpp b/rclcpp_action/src/client_base.cpp index 03bc5c2d7a..a823b65e92 100644 --- a/rclcpp_action/src/client_base.cpp +++ b/rclcpp_action/src/client_base.cpp @@ -181,7 +181,7 @@ class ClientBaseImpl std::map pending_cancel_responses; std::recursive_mutex cancel_requests_mutex; - std::recursive_mutex goal_id_rng_mutex; + std::mutex goal_id_rng_mutex; std::independent_bits_engine< std::mt19937, 8, unsigned int> random_bytes_generator; }; @@ -481,7 +481,7 @@ GoalUUID ClientBase::generate_goal_id() { GoalUUID goal_id; - std::lock_guard lock(pimpl_->goal_id_rng_mutex); + std::lock_guard lock(pimpl_->goal_id_rng_mutex); // TODO(hidmic): Do something better than this for UUID generation. // std::generate( // goal_id.uuid.begin(), goal_id.uuid.end(),