fix nullptr dereference in prune_requests_older_than (#2008) (#2065)

* fix nullptr dereference in prune_requests_older_than

Signed-off-by: akela1101 <akela1101@gmail.com>

* add tests for prune_requests_older_than

Signed-off-by: akela1101 <akela1101@gmail.com>

* Update rclcpp/test/rclcpp/test_client.cpp

Co-authored-by: Chen Lihui <lihui.chen@sony.com>
Signed-off-by: akela1101 <akela1101@gmail.com>

Signed-off-by: akela1101 <akela1101@gmail.com>
Co-authored-by: Chen Lihui <lihui.chen@sony.com>
(cherry picked from commit 1ac37b6)

Co-authored-by: andrei <akela1101@gmail.com>

rclcpp/include/rclcpp/client.hpp

@@ -769,7 +769,9 @@ class Client : public ClientBase
     auto old_size = pending_requests_.size();
     for (auto it = pending_requests_.begin(), last = pending_requests_.end(); it != last; ) {
       if (it->second.first < time_point) {
-        pruned_requests->push_back(it->first);
+        if (pruned_requests) {
+          pruned_requests->push_back(it->first);
+        }
         it = pending_requests_.erase(it);
       } else {
         ++it;

rclcpp/test/rclcpp/test_client.cpp

@@ -282,6 +282,27 @@ TEST_F(TestClientWithServer, test_client_remove_pending_request) {
   EXPECT_TRUE(client->remove_pending_request(future));
 }
 
+TEST_F(TestClientWithServer, prune_requests_older_than_no_pruned) {
+  auto client = node->create_client<test_msgs::srv::Empty>(service_name);
+  auto request = std::make_shared<test_msgs::srv::Empty::Request>();
+  auto future = client->async_send_request(request);
+  auto time = std::chrono::system_clock::now() + 1s;
+
+  EXPECT_EQ(1u, client->prune_requests_older_than(time));
+}
+
+TEST_F(TestClientWithServer, prune_requests_older_than_with_pruned) {
+  auto client = node->create_client<test_msgs::srv::Empty>(service_name);
+  auto request = std::make_shared<test_msgs::srv::Empty::Request>();
+  auto future = client->async_send_request(request);
+  auto time = std::chrono::system_clock::now() + 1s;
+
+  std::vector<int64_t> pruned_requests;
+  EXPECT_EQ(1u, client->prune_requests_older_than(time, &pruned_requests));
+  ASSERT_EQ(1u, pruned_requests.size());
+  EXPECT_EQ(future.request_id, pruned_requests[0]);
+}
+
 TEST_F(TestClientWithServer, async_send_request_rcl_send_request_error) {
   // Checking rcl_send_request in rclcpp::Client::async_send_request()
   auto mock = mocking_utils::patch_and_return("lib:rclcpp", rcl_send_request, RCL_RET_ERROR);