Skip to content

Commit

Permalink
fix: Fixed race condition in action server between is_ready and take_…
Browse files Browse the repository at this point in the history
…data and execute

Some background information: is_ready, take_data and execute data
may be called from different threads in any order. The code in the old
state expected them to be called in series, without interruption.
This lead to multiple race conditions, as the state of the pimpl objects
was altered by the three functions in a non thread safe way.

Signed-off-by: Janosch Machowinski <[email protected]>
Signed-off-by: William Woodall <[email protected]>

# Conflicts:
#	rclcpp_action/include/rclcpp_action/server.hpp
#	rclcpp_action/src/client.cpp
#	rclcpp_action/src/server.cpp
#	rclcpp_action/test/test_server.cpp
  • Loading branch information
Janosch Machowinski committed Apr 4, 2024
1 parent 9171835 commit 9b0d6de
Show file tree
Hide file tree
Showing 4 changed files with 409 additions and 245 deletions.
14 changes: 11 additions & 3 deletions rclcpp_action/include/rclcpp_action/server.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <unordered_map>
#include <utility>

#include "action_msgs/srv/cancel_goal.hpp"
#include "rcl/event_callback.h"
#include "rcl_action/action_server.h"
#include "rosidl_runtime_c/action_type_support_struct.h"
Expand Down Expand Up @@ -77,6 +78,7 @@ class ServerBase : public rclcpp::Waitable
GoalService,
ResultService,
CancelService,
Expired,
};

RCLCPP_ACTION_PUBLIC
Expand Down Expand Up @@ -279,19 +281,25 @@ class ServerBase : public rclcpp::Waitable
/// \internal
RCLCPP_ACTION_PUBLIC
void
execute_goal_request_received(const std::shared_ptr<void> & data);
execute_goal_request_received(
rcl_ret_t ret, rcl_action_goal_info_t goal_info, rmw_request_id_t request_header,
std::shared_ptr<void> message);

/// Handle a request to cancel goals on the server
/// \internal
RCLCPP_ACTION_PUBLIC
void
execute_cancel_request_received(const std::shared_ptr<void> & data);
execute_cancel_request_received(
rcl_ret_t ret, std::shared_ptr<action_msgs::srv::CancelGoal::Request> request,
rmw_request_id_t request_header);

/// Handle a request to get the result of an action
/// \internal
RCLCPP_ACTION_PUBLIC
void
execute_result_request_received(const std::shared_ptr<void> & data);
execute_result_request_received(
rcl_ret_t ret, std::shared_ptr<void> result_request,
rmw_request_id_t request_header);

/// Handle a timeout indicating a completed goal should be forgotten by the server
/// \internal
Expand Down
Loading

0 comments on commit 9b0d6de

Please sign in to comment.