[nav2_util/service_client.hpp] Potential memory leak when request times out

[oysstu]

https://github.com/ros-planning/navigation2/blob/b499116e017ccdbf13a8ba57c303aae54708dfc0/nav2_util/include/nav2_util/service_client.hpp#L79-L85

Came across this part of the service client code, and it seems like there may be a potential memory leak here if the spin_until_future_complete call times out. According to the documentation for async_send_request, Client::remove_pending_request must be called in the event that the executor stops spinning before the request has completed.

See documentation for async_send_request below
https://github.com/ros2/rclcpp/blob/cbd48c0eb481cdf5d17b2ca056d4596df8b0af1c/rclcpp/include/rclcpp/client.hpp#L587-L591

[SteveMacenski]

Thanks for the note! Is that something you’d be open to contributing a pull request to fix?

[oysstu]

Seems like an easy enough fix, I've created the PR. The docs do not explicitly mention what to do if FutureReturnCode::INTERRUPTED is returned, but seems likely that it also must be cleaned up since the request has not been processed. The remove_pending_request simply calls erase with the request id on an unordered map, which does nothing if the key doesn't exist.

[SteveMacenski]

Merged! Thanks for the note :-)