Skip to content

Commit 8f32c9a

Browse files
Janosch MachowinskiJanosch Machowinski
authored andcommitted
fix: Fixed race condition in action server between is_ready and take_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]>
1 parent 502334f commit 8f32c9a

File tree

4 files changed

+411
-246
lines changed

4 files changed

+411
-246
lines changed

rclcpp_action/include/rclcpp_action/server.hpp

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include <unordered_map>
2323
#include <utility>
2424

25+
#include "action_msgs/srv/cancel_goal.hpp"
2526
#include "rcl/event_callback.h"
2627
#include "rcl_action/action_server.h"
2728
#include "rosidl_runtime_c/action_type_support_struct.h"
@@ -77,6 +78,7 @@ class ServerBase : public rclcpp::Waitable
7778
GoalService,
7879
ResultService,
7980
CancelService,
81+
Expired,
8082
};
8183

8284
RCLCPP_ACTION_PUBLIC
@@ -279,19 +281,25 @@ class ServerBase : public rclcpp::Waitable
279281
/// \internal
280282
RCLCPP_ACTION_PUBLIC
281283
void
282-
execute_goal_request_received(std::shared_ptr<void> & data);
284+
execute_goal_request_received(
285+
rcl_ret_t ret, rcl_action_goal_info_t goal_info, rmw_request_id_t request_header,
286+
std::shared_ptr<void> message);
283287

284288
/// Handle a request to cancel goals on the server
285289
/// \internal
286290
RCLCPP_ACTION_PUBLIC
287291
void
288-
execute_cancel_request_received(std::shared_ptr<void> & data);
292+
execute_cancel_request_received(
293+
rcl_ret_t ret, std::shared_ptr<action_msgs::srv::CancelGoal::Request> request,
294+
rmw_request_id_t request_header);
289295

290296
/// Handle a request to get the result of an action
291297
/// \internal
292298
RCLCPP_ACTION_PUBLIC
293299
void
294-
execute_result_request_received(std::shared_ptr<void> & data);
300+
execute_result_request_received(
301+
rcl_ret_t ret, std::shared_ptr<void> result_request,
302+
rmw_request_id_t request_header);
295303

296304
/// Handle a timeout indicating a completed goal should be forgotten by the server
297305
/// \internal

0 commit comments

Comments
 (0)