Iron: Action IPC Fixes (#151)

* Fixes for intra-process actions (#144)

* Fixes for intra-process Actions

* Fixes for Clang builds

* Fix deadlock

* Server to store results until client requests them

* Fix feedback/result data race

See ros2#2451

* Add missing mutex

* Check return value of intra_process_action_send

---------

Co-authored-by: Mauro Passerino <[email protected]>

* Fix IPC Actions data race (#147)

* Check if goal was sent through IPC before send responses
* Add intra_process_action_server_is_available API to intra-process Client


---------

Co-authored-by: Mauro Passerino <[email protected]>

* Fix data race in Actions: Part 2 (#148)

* Fix data race in Actions: Part 2

* Fix warning - copy elision

---------

Co-authored-by: Mauro Passerino <[email protected]>

* fix: Fixed race condition in action server between is_ready and take"… (ros2#2531)

* fix: Fixed race condition in action server between is_ready and take" (ros2#2495)

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.

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

* fix: added workaround for call to double calls to take_data

This adds a workaround for a known bug in the executor in iron.

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

---------

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

---------

Signed-off-by: Janosch Machowinski <[email protected]>
Co-authored-by: Mauro Passerino <[email protected]>
Co-authored-by: jmachowinski <[email protected]>
Co-authored-by: Janosch Machowinski <[email protected]>
Co-authored-by: William Woodall <[email protected]>

Author: 5 people

rclcpp/include/rclcpp/experimental/intra_process_manager.hpp

@@ -1061,7 +1061,7 @@ class IntraProcessManager
           auto ptr = MessageAllocTraits::allocate(allocator, 1);
           MessageAllocTraits::construct(allocator, ptr, *message);
 
-          subscription->provide_intra_process_data(std::move(MessageUniquePtr(ptr, deleter)));
+          subscription->provide_intra_process_data(MessageUniquePtr(ptr, deleter));
         }
 
         continue;
@@ -1104,7 +1104,7 @@ class IntraProcessManager
             MessageAllocTraits::construct(allocator, ptr, *message);
 
             ros_message_subscription->provide_intra_process_message(
-              std::move(MessageUniquePtr(ptr, deleter)));
+              MessageUniquePtr(ptr, deleter));
           }
         }
       }

rclcpp_action/include/rclcpp_action/client.hpp

@@ -97,6 +97,16 @@ class ClientBase : public rclcpp::Waitable
     );
   }
 
+  /// Return true if there is an intra-process action server that is ready to take goal requests.
+  bool
+  intra_process_action_server_is_available()
+  {
+    if (auto ipm = weak_ipm_.lock()) {
+      return ipm->action_server_is_available(ipc_action_client_id_);
+    }
+    return false;
+  }
+
   // -------------
   // Waitables API
 
@@ -835,12 +845,18 @@ class Client : public ClientBase
         // the server might be available in another process or was configured to not use IPC.
         if (intra_process_server_available) {
           size_t hashed_guuid = std::hash<GoalUUID>()(goal_handle->get_goal_id());
-          ipc_action_client_->store_result_response_callback(
-            hashed_guuid, result_response_callback);
 
-          intra_process_send_done = ipm->template intra_process_action_send_result_request<ActionT>(
-              ipc_action_client_id_,
-              std::move(goal_result_request));
+          // Determine if goal was sent through inter or intra process by checking the goal ID
+          bool goal_sent_by_ipc = ipm->get_action_client_id_from_goal_uuid(hashed_guuid);
+
+          if (goal_sent_by_ipc) {
+            ipc_action_client_->store_result_response_callback(
+              hashed_guuid, result_response_callback);
+
+            intra_process_send_done = ipm->template intra_process_action_send_result_request<ActionT>(
+                ipc_action_client_id_,
+                std::move(goal_result_request));
+          }
         }
       }
 

rclcpp_action/include/rclcpp_action/server.hpp

@@ -675,7 +675,8 @@ class Server : public ServerBase, public std::enable_shared_from_this<Server<Act
       rclcpp::get_logger("rclcpp_action"),
       "Action server can't send result response, missing IPC Action client: %s. "
       "Will do inter-process publish",
-      this->action_name_.c_str());
+      this->action_name_);
+
     return true;
   }