Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory leak from strdup #1198

Open
jimmy-mcelwain opened this issue Nov 15, 2024 · 1 comment
Open

Memory leak from strdup #1198

jimmy-mcelwain opened this issue Nov 15, 2024 · 1 comment
Assignees
Labels
more-information-needed Further information is required

Comments

@jimmy-mcelwain
Copy link

Host: YRC1000 running MotoROS2
Client: PC running Ubuntu 22.04
ROS2 version: Humble
Commits: The commit hashes/version used to build MotoROS2 are available here
DDS: FastDDS

I am a contributor to MotoROS2. Our implementation allows the controller and client to be disconnected/reconnected repeatedly. Whenever I disconnect the client pc from the host, the memory is freed and everything is shut down, and then everything is re-initialized upon connecting again. However, I noticed a memory leak, 24 bytes per disconnect/reconnect cycle. I eventually traced it to its source and found the leak.

rcl/rcl/src/rcl/init.c

Lines 216 to 223 in 2615b0e

if (context->global_arguments.impl->enclave) {
context->impl->init_options.impl->rmw_init_options.enclave = rcutils_strdup(
context->global_arguments.impl->enclave,
context->impl->allocator);
} else {
context->impl->init_options.impl->rmw_init_options.enclave = rcutils_strdup(
"/", context->impl->allocator);
}

The memory allocated in these strdup calls is never freed.

I have a branch on our fork of micro-ros here where I added a single line freeing the memory allocated in that strdup, and the memory leak disappeared. There is now no memory lost with a connect/disconnect cycle.

I noticed the problem in Motoman's own fork of micro-ros/rcl, which is somewhat outdated. But looking through the up-to-date upstream repos, the problem is seemingly present in micro-ros/rcl as well as here. Not just in humble, but in newer distributions/rolling as well.

I do not know if the spot that I freed the memory is the proper spot to do so. But if I understand correctly, it has not yet been fixed upstream (here) and the proper spot to free the memory is somewhere in this repo.

@fujitatomoya
Copy link
Collaborator

which is somewhat outdated. But looking through the up-to-date upstream repos, the problem is seemingly present in micro-ros/rcl as well as here. Not just in humble, but in newer distributions/rolling as well.

did you confirm the there is a memory leak on ros2/rcl (rolling)?

it looks like it frees the memory here.

rcl_ret_t init_options_fini_ret = rcl_init_options_fini(&(context->impl->init_options));

rmw_ret_t rmw_ret = rmw_init_options_fini(&(init_options->impl->rmw_init_options));

and i see underlying all rmw tier 1 implementation free the memory for enclave.

@fujitatomoya fujitatomoya self-assigned this Nov 16, 2024
@fujitatomoya fujitatomoya added the more-information-needed Further information is required label Nov 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
more-information-needed Further information is required
Projects
None yet
Development

No branches or pull requests

2 participants