Skip to content

Commit 3a78265

Browse files
author
Mauro Passerino
committed
Fix data race in Actions: Part 2
1 parent f165a25 commit 3a78265

File tree

2 files changed

+11
-30
lines changed

2 files changed

+11
-30
lines changed

rclcpp/include/rclcpp/experimental/action_client_intra_process_base.hpp

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -158,17 +158,6 @@ class ActionClientIntraProcessBase : public rclcpp::Waitable
158158
event_info_multi_map_.erase(goal_id);
159159
}
160160

161-
bool has_goal_id(size_t goal_id) const
162-
{
163-
// Check if the intra-process client has this goal_id
164-
auto it = event_info_multi_map_.find(goal_id);
165-
if (it != event_info_multi_map_.end()) {
166-
return true;
167-
}
168-
169-
return false;
170-
}
171-
172161
private:
173162
std::string action_name_;
174163
QoS qos_profile_;

rclcpp_action/include/rclcpp_action/client.hpp

Lines changed: 11 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -523,17 +523,13 @@ class Client : public ClientBase
523523
// If there's not, we fall back into inter-process communication, since
524524
// the server might be available in another process or was configured to not use IPC.
525525
if (intra_process_server_available) {
526-
bool goal_sent_by_ipc = ipc_action_client_->has_goal_id(hashed_guuid);
526+
ipc_action_client_->store_goal_response_callback(
527+
hashed_guuid, goal_response_callback);
527528

528-
if (goal_sent_by_ipc) {
529-
ipc_action_client_->store_goal_response_callback(
530-
hashed_guuid, goal_response_callback);
531-
532-
intra_process_send_done = ipm->template intra_process_action_send_goal_request<ActionT>(
533-
ipc_action_client_id_,
534-
std::move(goal_request),
535-
hashed_guuid);
536-
}
529+
intra_process_send_done = ipm->template intra_process_action_send_goal_request<ActionT>(
530+
ipc_action_client_id_,
531+
std::move(goal_request),
532+
hashed_guuid);
537533
}
538534
}
539535

@@ -850,7 +846,8 @@ class Client : public ClientBase
850846
if (intra_process_server_available) {
851847
size_t hashed_guuid = std::hash<GoalUUID>()(goal_handle->get_goal_id());
852848

853-
bool goal_sent_by_ipc = ipc_action_client_->has_goal_id(hashed_guuid);
849+
// Determine if goal was sent through inter or intra process by checking the goal ID
850+
bool goal_sent_by_ipc = ipm->get_action_client_id_from_goal_uuid(hashed_guuid);
854851

855852
if (goal_sent_by_ipc) {
856853
ipc_action_client_->store_result_response_callback(
@@ -910,17 +907,12 @@ class Client : public ClientBase
910907
// the server might be available in another process or was configured to not use IPC.
911908
if (intra_process_server_available) {
912909
size_t hashed_guuid = std::hash<GoalUUID>()(cancel_request->goal_info.goal_id.uuid);
910+
ipc_action_client_->store_cancel_goal_callback(
911+
hashed_guuid, cancel_goal_callback);
913912

914-
bool goal_sent_by_ipc = ipc_action_client_->has_goal_id(hashed_guuid);
915-
916-
if (goal_sent_by_ipc) {
917-
ipc_action_client_->store_cancel_goal_callback(
918-
hashed_guuid, cancel_goal_callback);
919-
920-
intra_process_send_done = ipm->template intra_process_action_send_cancel_request<ActionT>(
913+
intra_process_send_done = ipm->template intra_process_action_send_cancel_request<ActionT>(
921914
ipc_action_client_id_,
922915
std::move(cancel_request));
923-
}
924916
}
925917
}
926918

0 commit comments

Comments
 (0)