issue-919 Fixed "memory leak" in action clients (#920)

Whenever a call is made to `rclcpp_action::Client::wait_for_action_server`
a weak pointer to an Event object gets added to the graph_event_ vector
of the NodeGraph interface. This vector will be cleared on a node graph
change event, but if no such event occurs the weak pointer will be stuck
in the vector.  Furthermore, if client code issues repeated calls to
`wait_for_action_server` the vector will keep growing.

The fix moves the Event object creation right after the early return from
`wait_for_action_server` so that the Event object is not created in the
case that the server is known to be present and therefore there is no
need to wait for a node graph change event to occur.

Signed-off-by: Adrian Stere <astere@clearpath.ai>
This commit is contained in:
astere-cpr 2019-11-19 14:09:41 -05:00 committed by Ivan Santiago Paunovic
parent a512d58a4f
commit ecc39cace6

View file

@ -165,11 +165,11 @@ ClientBase::wait_for_action_server_nanoseconds(std::chrono::nanoseconds timeout)
if (!node_ptr) { if (!node_ptr) {
throw rclcpp::exceptions::InvalidNodeError(); throw rclcpp::exceptions::InvalidNodeError();
} }
auto event = node_ptr->get_graph_event();
// check to see if the server is ready immediately // check to see if the server is ready immediately
if (this->action_server_is_ready()) { if (this->action_server_is_ready()) {
return true; return true;
} }
auto event = node_ptr->get_graph_event();
if (timeout == std::chrono::nanoseconds(0)) { if (timeout == std::chrono::nanoseconds(0)) {
// check was non-blocking, return immediately // check was non-blocking, return immediately
return false; return false;