Skip to content

Commit 690fa75

Browse files
author
Prajakta Gokhale
committed
Fix leaks in rcl_action unit tests
Fix memory leaks detected by AddressSanitizer from rcl_action unit tests. Signed-off-by: Prajakta Gokhale <[email protected]>
1 parent 0c4c743 commit 690fa75

File tree

7 files changed

+40
-7
lines changed

7 files changed

+40
-7
lines changed

‎rcl/src/rcl/init.c‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,13 @@ rcl_shutdown(rcl_context_t * context)
179179
return rcl_convert_rmw_ret_to_rcl_ret(rmw_ret);
180180
}
181181

182-
rcl_ret_t rcl_ret = rcl_logging_fini();
182+
rcl_ret_t rcl_ret = rcl_context_fini(context);
183+
if (RCL_RET_OK != rcl_ret) {
184+
RCUTILS_LOG_ERROR_NAMED(ROS_PACKAGE_NAME, "Failed to fini context");
185+
return rcl_ret;
186+
}
187+
188+
rcl_ret = rcl_logging_fini();
183189
RCUTILS_LOG_ERROR_EXPRESSION_NAMED(RCL_RET_OK != rcl_ret, ROS_PACKAGE_NAME,
184190
"Failed to fini logging, rcl_ret_t: %i, rcl_error_str: %s", rcl_ret,
185191
rcl_get_error_string().str);

‎rcl_action/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,7 @@ if(BUILD_TESTING)
170170
${PROJECT_NAME}
171171
)
172172
ament_target_dependencies(test_action_server
173+
"osrf_testing_tools_cpp"
173174
"rcl"
174175
"test_msgs"
175176
)

‎rcl_action/test/rcl_action/test_action_client.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ class TestActionClientBaseFixture : public ::testing::Test
5252
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
5353
ret = rcl_shutdown(&this->context);
5454
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
55+
ret = rcl_context_fini(&this->context);
56+
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
5557
}
5658

5759
rcl_context_t context;

‎rcl_action/test/rcl_action/test_action_communication.cpp‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
// limitations under the License.
1414
#include <gtest/gtest.h>
1515

16+
#include "osrf_testing_tools_cpp/scope_exit.hpp"
17+
1618
#include "rcl_action/action_client.h"
1719
#include "rcl_action/action_server.h"
1820
#include "rcl_action/wait.h"
@@ -41,6 +43,9 @@ class CLASSNAME (TestActionCommunication, RMW_IMPLEMENTATION) : public ::testing
4143
rcl_init_options_t init_options = rcl_get_zero_initialized_init_options();
4244
ret = rcl_init_options_init(&init_options, allocator);
4345
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
46+
OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({
47+
EXPECT_EQ(RCL_RET_OK, rcl_init_options_fini(&init_options)) << rcl_get_error_string().str;
48+
});
4449
context = rcl_get_zero_initialized_context();
4550
ret = rcl_init(0, nullptr, &init_options, &context);
4651
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
@@ -119,6 +124,8 @@ class CLASSNAME (TestActionCommunication, RMW_IMPLEMENTATION) : public ::testing
119124
EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
120125
ret = rcl_shutdown(&context);
121126
EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
127+
ret = rcl_context_fini(&this->context);
128+
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
122129
}
123130

124131
void init_test_uuid0(uint8_t * uuid)

‎rcl_action/test/rcl_action/test_action_interaction.cpp‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
// limitations under the License.
1414
#include <gtest/gtest.h>
1515

16+
#include "osrf_testing_tools_cpp/scope_exit.hpp"
17+
1618
#include "rcl_action/action_client.h"
1719
#include "rcl_action/action_server.h"
1820
#include "rcl_action/wait.h"
@@ -53,6 +55,9 @@ class CLASSNAME (TestActionClientServerInteraction, RMW_IMPLEMENTATION) : public
5355
rcl_init_options_t init_options = rcl_get_zero_initialized_init_options();
5456
ret = rcl_init_options_init(&init_options, allocator);
5557
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
58+
OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({
59+
EXPECT_EQ(RCL_RET_OK, rcl_init_options_fini(&init_options)) << rcl_get_error_string().str;
60+
});
5661
context = rcl_get_zero_initialized_context();
5762
ret = rcl_init(0, nullptr, &init_options, &context);
5863
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
@@ -141,6 +146,8 @@ class CLASSNAME (TestActionClientServerInteraction, RMW_IMPLEMENTATION) : public
141146
EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
142147
ret = rcl_shutdown(&context);
143148
EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
149+
ret = rcl_context_fini(&this->context);
150+
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
144151
}
145152

146153
void init_test_uuid0(uint8_t * uuid)

‎rcl_action/test/rcl_action/test_action_server.cpp‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919

2020
#include "action_msgs/srv/cancel_goal.h"
2121

22+
#include "osrf_testing_tools_cpp/scope_exit.hpp"
23+
2224
#include "rcl_action/action_server.h"
2325

2426
#include "rcl/error_handling.h"
@@ -137,12 +139,21 @@ TEST(TestActionServerInitFini, test_action_server_init_fini)
137139
ret = rcl_clock_fini(&clock);
138140
EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
139141

142+
// Finalize init_options
143+
ret = rcl_init_options_fini(&init_options);
144+
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
145+
140146
// Finalize node
141147
ret = rcl_node_fini(&node);
142148
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
143149

150+
// Shutdown node
144151
ret = rcl_shutdown(&context);
145152
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
153+
154+
// Finalize context
155+
ret = rcl_context_fini(&context);
156+
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
146157
}
147158

148159
class TestActionServer : public ::testing::Test
@@ -155,6 +166,9 @@ class TestActionServer : public ::testing::Test
155166
rcl_init_options_t init_options = rcl_get_zero_initialized_init_options();
156167
ret = rcl_init_options_init(&init_options, allocator);
157168
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
169+
OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({
170+
EXPECT_EQ(RCL_RET_OK, rcl_init_options_fini(&init_options)) << rcl_get_error_string().str;
171+
});
158172
context = rcl_get_zero_initialized_context();
159173
ret = rcl_init(0, nullptr, &init_options, &context);
160174
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
@@ -185,6 +199,8 @@ class TestActionServer : public ::testing::Test
185199
EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
186200
ret = rcl_shutdown(&context);
187201
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
202+
ret = rcl_context_fini(&this->context);
203+
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
188204
}
189205

190206
void init_test_uuid0(uint8_t * uuid)

‎rcl_action/test/rcl_action/test_graph.cpp‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -94,10 +94,6 @@ class CLASSNAME (TestActionGraphFixture, RMW_IMPLEMENTATION) : public ::testing:
9494

9595
ret = rcl_shutdown(&this->context);
9696
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
97-
ret = rcl_context_fini(&this->context);
98-
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
99-
ret = rcl_context_fini(&this->old_context);
100-
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
10197
}
10298
};
10399

@@ -320,8 +316,6 @@ class TestActionGraphMultiNodeFixture : public CLASSNAME(TestActionGraphFixture,
320316

321317
ret = rcl_shutdown(&this->remote_context);
322318
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
323-
ret = rcl_context_fini(&this->remote_context);
324-
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
325319
}
326320

327321
void WaitForAllNodesAlive()

0 commit comments

Comments
 (0)