memory leaks in rcl(tests) #721
Comments
|
I'd like to contribute to this issue. |
|
I found there are four issues in your descriptions.
What do you think about above ideas? If you have some suggestions, could you please share them with me? |
|
I'll try to find a method to solve item |
|
About item |
I confirmed that we don't see this memory leak with ros2/ros2@91d8a43 |
|
@iuhilnehc-ynos
I agree on this. if the message taken via I did confirm that following patch can avert the memory leak problem observed in valgrind. would you create PR? |
|
Great that you've found already some potential improvements. Feel free to open a PR on the related repository and link them to this issue. |
Yes, I'll create it later. |
|
i think you can close this issue, if we are missing something, let us know. |
|
Actually, I am ready to fix all tests in make it short
./test_lexer_lookahead__rmw_fastrtps_dynamic_cpp
./test_subscription__rmw_cyclonedds_cpp
./test_info_by_topic__rmw_cyclonedds_cpp
./service_fixture__rmw_fastrtps_cpp
./test_rmw_impl_id_check_exe
./test_logging_rosout__rmw_fastrtps_dynamic_cpp
./test_timer__rmw_fastrtps_cpp
./service_fixture__rmw_fastrtps_dynamic_cpp
./test_lexer_lookahead__rmw_cyclonedds_cpp
./test_rmw_impl_id_check_func__rmw_cyclonedds_cpp
./test_count_matched__rmw_fastrtps_dynamic_cpp
./test_subscription__rmw_fastrtps_cpp
./client_fixture__rmw_fastrtps_cpp
./test_publisher__rmw_fastrtps_dynamic_cpp
./test_context__rmw_fastrtps_dynamic_cpp
./test_service__rmw_fastrtps_dynamic_cpp
./test_service__rmw_cyclonedds_cpp
./test_arguments__rmw_cyclonedds_cpp
./test_get_node_names__rmw_fastrtps_dynamic_cpp
./test_lexer__rmw_fastrtps_dynamic_cpp
./test_context__rmw_cyclonedds_cpp
./test_info_by_topic__rmw_fastrtps_cpp
./test_timer__rmw_cyclonedds_cpp
./test_publisher__rmw_fastrtps_cpp
./test_node__rmw_fastrtps_dynamic_cpp
./test_publisher__rmw_cyclonedds_cpp
./test_arguments__rmw_fastrtps_cpp
./test_timer__rmw_fastrtps_dynamic_cpp
./test_client__rmw_cyclonedds_cpp
./test_namespace__rmw_fastrtps_dynamic_cpp
./test_info_by_topic__rmw_fastrtps_dynamic_cpp
./test_namespace__rmw_fastrtps_cpp
./test_graph__rmw_fastrtps_cpp
./test_rmw_impl_id_check_func__rmw_fastrtps_dynamic_cpp
./test_validate_enclave_name
./test_common
./test_node__rmw_fastrtps_cpp
./test_guard_condition__rmw_fastrtps_dynamic_cpp
./test_arguments__rmw_fastrtps_dynamic_cpp
./test_remap__rmw_fastrtps_cpp
./test_get_actual_qos__rmw_fastrtps_dynamic_cpp
./test_node__rmw_cyclonedds_cpp
./test_events__rmw_fastrtps_cpp
./test_time__rmw_cyclonedds_cpp
./test_time__rmw_fastrtps_cpp
./test_wait__rmw_fastrtps_cpp
./test_validate_topic_name
./test_remap__rmw_cyclonedds_cpp
./test_lexer_lookahead__rmw_fastrtps_cpp
./test_remap_integration__rmw_fastrtps_dynamic_cpp
./test_get_node_names__rmw_cyclonedds_cpp
./test_remap_integration__rmw_fastrtps_cpp
./test_security
./test_init__rmw_fastrtps_cpp
./test_rmw_impl_id_check_func__rmw_fastrtps_cpp
./test_context__rmw_fastrtps_cpp
./test_wait__rmw_cyclonedds_cpp
./test_log_level
./test_expand_topic_name
./test_localhost
./test_wait__rmw_fastrtps_dynamic_cpp
./test_remap_integration__rmw_cyclonedds_cpp
./test_graph__rmw_cyclonedds_cpp
./test_get_actual_qos__rmw_fastrtps_cpp
./client_fixture__rmw_fastrtps_dynamic_cpp
./test_client__rmw_fastrtps_dynamic_cpp
./test_time__rmw_fastrtps_dynamic_cpp
./test_lexer__rmw_cyclonedds_cpp
./test_events__rmw_cyclonedds_cpp
./client_fixture__rmw_cyclonedds_cpp
./test_client__rmw_fastrtps_cpp
./test_guard_condition__rmw_cyclonedds_cpp
./test_domain_id
./test_remap__rmw_fastrtps_dynamic_cpp
./test_logging_rosout__rmw_fastrtps_cpp
./test_count_matched__rmw_cyclonedds_cpp
./test_init__rmw_cyclonedds_cpp
./test_guard_condition__rmw_fastrtps_cpp
./test_subscription__rmw_fastrtps_dynamic_cpp
./service_fixture__rmw_cyclonedds_cpp
./test_lexer__rmw_fastrtps_cpp
./test_namespace__rmw_cyclonedds_cpp
./test_get_node_names__rmw_fastrtps_cpp
./test_logging
./test_init__rmw_fastrtps_dynamic_cpp
./test_graph__rmw_fastrtps_dynamic_cpp
./test_count_matched__rmw_fastrtps_cpp
./test_get_actual_qos__rmw_cyclonedds_cpp
./test_logging_rosout__rmw_cyclonedds_cpp
./test_service__rmw_fastrtps_cpp
./test_events__rmw_fastrtps_dynamic_cpp |
|
in that case, we should make another issue to address? i thought this issue is to fix the #721 (comment). i think that it would be nice to keep the history clearly. and maybe the problem could not be |
|
I was thinking to make a new issue and then relate it to this manager issue if I found that memory leaks in a specific test executable file.
I agree with it. |
|
@Karsten1987 @fujitatomoya @iuhilnehc-ynos Based master branch, we find memory leak in below test cases.
|
|
@Barry-Xu-2018 sounds good to me, thanks. but maybe maintainer would prefer to create dedicated issue. |
|
@iuhilnehc-ynos and I have finished analyzing all memory leak cases in above table. |
|
The cause for test_wait__rmw_cyclonedds_cpp／test_wait__rmw_fastrtps_cpp／ test_wait__rmw_fastrtps_dynamic_cpp The test leading memory leak is rcl/rcl/test/rcl/test_wait.cpp Lines 96 to 113 in 8bcada7 After call Lines 385 to 391 in 8bcada7 For SET_RESIZE_RMW_DEALLOC, It tries to free allocated memory but fails. Lines 288 to 294 in 8bcada7 allocator.reallocate() return fail. And count is set to 0. This leads memory leak. Lines 296 to 307 in 8bcada7 If modify below 2 points in test_wait.cpp, memory leak can be avoided. void *
failing_realloc(void * pointer, size_t size, void * state)
{
if (((__failing_allocator_state *)state)->is_failing) {
// New added below line to make sure free allocated memory
rcutils_get_default_allocator().deallocate(pointer, rcutils_get_default_allocator().state);
return nullptr;
}
return rcutils_get_default_allocator().reallocate(
pointer, size, rcutils_get_default_allocator().state);
}
void
failing_free(void * pointer, void * state)
{
#if 0 // Avoid exiting to make sure free allocated memory
if (((__failing_allocator_state *)state)->is_failing) {
return;
}
#endif
RCUTILS_UNUSED(state);
rcutils_get_default_allocator().deallocate(pointer, rcutils_get_default_allocator().state);
} |
|
All analysis has been finished. |
I'd like to hear from others though. |
Refer to the manual of remalloc(), there is below sentence So failing_realloc() doesn't free original block is the same behavior like realloc(). |
|
okay, i think memory leak could be acceptable only in test with purpose, if we keep the consistency of failing functions. anyway, we can wait for the comments from other people on this! |
|
i think we can close this, since #721 (comment) all problems have been addressed. |
|
ping friendly. |
|
I'm going to go ahead and close this; please feel free to reopen if that is in error. Thanks for all of the work fixing this! |
|
@clalancette thanks for checking on this! |
while debugging #715 I came across that quite some tests are actually leaking memory. I haven't looked into whether these are related to the tests or actually something within rcl itself.
for
test_node__rmw_fastrtps_cpp:for comparison, the same test with cyclonedds:
The text was updated successfully, but these errors were encountered: