Skip to content

Commit

Permalink
Fix remaining leaks in test_action_server
Browse files Browse the repository at this point in the history
  • Loading branch information
@sloretz
sloretz committed Nov 29, 2018
1 parent 0a9c9ac commit a804d26
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 0 deletions.
1 change: 1 addition & 0 deletions rcl_action/src/rcl_action/action_server.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -645,6 +645,7 @@ rcl_action_expire_goals(
// Fill in any gaps left in the array with pointers from the end
--num_goal_handles;
action_server->impl->goal_handles[i] = action_server->impl->goal_handles[num_goal_handles];
allocator.deallocate(action_server->impl->goal_handles[num_goal_handles], allocator.state);
action_server->impl->goal_handles[num_goal_handles] = NULL;
++num_goals_expired;
}
Expand Down
26 changes: 26 additions & 0 deletions rcl_action/test/rcl_action/test_action_server.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,9 +233,12 @@ TEST_F(TestActionServer, test_action_accept_new_goal)
EXPECT_EQ(goal_handle, nullptr);
rcl_reset_error();

std::vector<rcl_action_goal_handle_t> handles;

// Accept with valid arguments
goal_handle = rcl_action_accept_new_goal(&this->action_server, &goal_info_in);
EXPECT_NE(goal_handle, nullptr) << rcl_get_error_string().str;
handles.push_back(*goal_handle);
rcl_action_goal_info_t goal_info_out = rcl_action_get_zero_initialized_goal_info();
rcl_ret_t ret = rcl_action_goal_handle_get_info(goal_handle, &goal_info_out);
ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
Expand All @@ -258,6 +261,7 @@ TEST_F(TestActionServer, test_action_accept_new_goal)
init_test_uuid1(goal_info_in.uuid);
goal_handle = rcl_action_accept_new_goal(&this->action_server, &goal_info_in);
EXPECT_NE(goal_handle, nullptr) << rcl_get_error_string().str;
handles.push_back(*goal_handle);
ret = rcl_action_goal_handle_get_info(goal_handle, &goal_info_out);
ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
EXPECT_TRUE(uuidcmp(goal_info_out.uuid, goal_info_in.uuid));
Expand All @@ -267,6 +271,10 @@ TEST_F(TestActionServer, test_action_accept_new_goal)
EXPECT_NE(goal_handle_array, nullptr) << rcl_get_error_string().str;
EXPECT_NE(goal_handle_array[0], nullptr) << rcl_get_error_string().str;
EXPECT_NE(goal_handle_array[1], nullptr) << rcl_get_error_string().str;

for (auto & handle : handles) {
EXPECT_EQ(RCL_RET_OK, rcl_action_goal_handle_fini(&handle));
}
}

TEST_F(TestActionServer, test_action_clear_expired_goals)
Expand Down Expand Up @@ -294,6 +302,8 @@ TEST_F(TestActionServer, test_action_clear_expired_goals)
ret = rcl_action_expire_goals(&this->action_server, nullptr, 0u, nullptr);
EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;

std::vector<rcl_action_goal_handle_t> handles;

// Test with goals that actually expire
// Set ROS time
ASSERT_EQ(RCL_RET_OK, rcl_enable_ros_time_override(&this->clock));
Expand All @@ -304,6 +314,7 @@ TEST_F(TestActionServer, test_action_clear_expired_goals)
rcl_action_goal_handle_t * goal_handle =
rcl_action_accept_new_goal(&this->action_server, &goal_info_in);
ASSERT_NE(goal_handle, nullptr) << rcl_get_error_string().str;
handles.push_back(*goal_handle);
// Transition executing to aborted
ASSERT_EQ(RCL_RET_OK, rcl_action_update_goal_state(goal_handle, GOAL_EVENT_EXECUTE));
ASSERT_EQ(RCL_RET_OK, rcl_action_update_goal_state(goal_handle, GOAL_EVENT_SET_ABORTED));
Expand All @@ -314,6 +325,10 @@ TEST_F(TestActionServer, test_action_clear_expired_goals)
EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
EXPECT_EQ(num_expired, 1u);
EXPECT_TRUE(uuidcmp(expired_goals[0].uuid, goal_info_in.uuid));

for (auto & handle : handles) {
EXPECT_EQ(RCL_RET_OK, rcl_action_goal_handle_fini(&handle));
}
}

TEST_F(TestActionServer, test_action_process_cancel_request)
Expand Down Expand Up @@ -379,12 +394,15 @@ TEST_F(TestActionServer, test_action_server_get_goal_status_array)
ret = rcl_action_goal_status_array_fini(&status_array);
ASSERT_EQ(ret, RCL_RET_OK);

std::vector<rcl_action_goal_handle_t> handles;

// Add a goal before getting the status array
rcl_action_goal_info_t goal_info_in = rcl_action_get_zero_initialized_goal_info();
init_test_uuid0(goal_info_in.uuid);
rcl_action_goal_handle_t * goal_handle;
goal_handle = rcl_action_accept_new_goal(&this->action_server, &goal_info_in);
ASSERT_NE(goal_handle, nullptr) << rcl_get_error_string().str;
handles.push_back(*goal_handle);
ret = rcl_action_get_goal_status_array(&this->action_server, &status_array);
EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
EXPECT_NE(status_array.msg.status_list.data, nullptr);
Expand All @@ -401,6 +419,7 @@ TEST_F(TestActionServer, test_action_server_get_goal_status_array)
}
goal_handle = rcl_action_accept_new_goal(&this->action_server, &goal_info_in);
ASSERT_NE(goal_handle, nullptr) << rcl_get_error_string().str;
handles.push_back(*goal_handle);
}
ret = rcl_action_get_goal_status_array(&this->action_server, &status_array);
EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
Expand All @@ -414,6 +433,9 @@ TEST_F(TestActionServer, test_action_server_get_goal_status_array)
}
ret = rcl_action_goal_status_array_fini(&status_array);
ASSERT_EQ(ret, RCL_RET_OK);
for (auto & handle : handles) {
EXPECT_EQ(RCL_RET_OK, rcl_action_goal_handle_fini(&handle));
}
}

TEST_F(TestActionServer, test_action_server_get_action_name)
Expand Down Expand Up @@ -510,6 +532,7 @@ TEST_F(TestActionServerCancelPolicy, test_action_process_cancel_request_all_goal
EXPECT_EQ(goal_info_out->uuid[j], static_cast<uint8_t>(i + j));
}
}
EXPECT_EQ(RCL_RET_OK, rcl_action_cancel_response_fini(&cancel_response));
}

TEST_F(TestActionServerCancelPolicy, test_action_process_cancel_request_single_goal)
Expand All @@ -525,6 +548,7 @@ TEST_F(TestActionServerCancelPolicy, test_action_process_cancel_request_single_g
ASSERT_EQ(cancel_response.msg.goals_canceling.size, 1u);
rcl_action_goal_info_t * goal_info = &cancel_response.msg.goals_canceling.data[0];
EXPECT_TRUE(uuidcmp(goal_info->uuid, cancel_request.goal_info.uuid));
EXPECT_EQ(RCL_RET_OK, rcl_action_cancel_response_fini(&cancel_response));
}

TEST_F(TestActionServerCancelPolicy, test_action_process_cancel_request_by_time)
Expand All @@ -546,6 +570,7 @@ TEST_F(TestActionServerCancelPolicy, test_action_process_cancel_request_by_time)
EXPECT_EQ(goal_info_out->uuid[j], static_cast<uint8_t>(i + j));
}
}
EXPECT_EQ(RCL_RET_OK, rcl_action_cancel_response_fini(&cancel_response));
}

TEST_F(TestActionServerCancelPolicy, test_action_process_cancel_request_by_time_and_id)
Expand Down Expand Up @@ -574,4 +599,5 @@ TEST_F(TestActionServerCancelPolicy, test_action_process_cancel_request_by_time_
}
goal_info_out = &cancel_response.msg.goals_canceling.data[num_goals_canceling - 1];
EXPECT_TRUE(uuidcmp(goal_info_out->uuid, cancel_request.goal_info.uuid));
EXPECT_EQ(RCL_RET_OK, rcl_action_cancel_response_fini(&cancel_response));
}

0 comments on commit a804d26

Please sign in to comment.