Here is a MWE. It's a silly program that doesn't accomplish anything other than to demonstrate this problem, but when I use valgrind I get the following results:

....
==18276== 222 bytes in 111 blocks are definitely lost in loss record 17 of 18
==18276==    at 0x4848899: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==18276==    by 0x492E85C: rcutils_strndup (in /opt/ros/humble/lib/librcutils.so)
==18276==    by 0x495382D: rcl_init (in /opt/ros/humble/lib/librcl.so)
==18276==    by 0x48613D5: rclc_support_init_with_options (in /opt/ros/humble/lib/librclc.so)
==18276==    by 0x10942D: Ros_StateServer_TimerCallback (in /home/jimmy/everything_motoros2/Other/reproduce/install/lib/reproduce/reproduce)
==18276==    by 0x4961679: rcl_timer_call (in /opt/ros/humble/lib/librcl.so)
==18276==    by 0x4866078: ??? (in /opt/ros/humble/lib/librclc.so)
==18276==    by 0x486B0BF: rclc_executor_spin_some (in /opt/ros/humble/lib/librclc.so)
==18276==    by 0x486C0AE: rclc_executor_spin (in /opt/ros/humble/lib/librclc.so)
==18276==    by 0x109771: main (in /home/jimmy/everything_motoros2/Other/reproduce/install/lib/reproduce/reproduce)
.....

This suggests that it is the rcutils_strdup in rcl_init

@jimmy-mcelwain thanks for recreating the issue here, this does seem like a problem.

I would need to check this deeper but the for of rcl that we maintain for micro-ROS is intended to be built with RCL_MICROROS_COMPLETE_IMPL to not defined. In a sense, this flag excludes the code that micro-ROS does not need from mainstream rcl.

In any case, I wonder why rmw is supposed to deallocate memory being allocated by rcl?

I would need to check this deeper but the for of rcl that we maintain for micro-ROS is intended to be built with RCL_MICROROS_COMPLETE_IMPL to not defined. In a sense, this flag excludes the code that micro-ROS does not need from mainstream rcl.

we understand.

But seeing as rcl maintainers suggest this is an issue with the RMW, we thought we'd report it anyway.

In any case, I wonder why rmw is supposed to deallocate memory being allocated by rcl?

That is also something I'm not sure about.

Perhaps @clalancette and/or @fujitatomoya can say something about that.

Honestly, this seems like a bug in rcl (or, at least, in the expectations between rcl and rmw). I agree that it doesn't make sense for the rcl layer to allocate it, and the rmw layer to free it.

To fix it, we have to either:

1. Leave the strndup in rcl_init, move the free into __cleanup_context, and remove the free from all of the RMW implementations, or
2. Move the strndup out of rcl_init and into rmw_init, and leave the free in the RMW.

I would actually probably go for the latter; while it is annoying to have to duplicate that code among the various RMWs, I think it also makes more sense since this code is manipulating the rmw_init_options structure. But I don't have a strong opinion either way.

In any case, I wonder why rmw is supposed to deallocate memory being allocated by rcl?

yeah, thanks for pointing this out, i focused on memory leak issue. this is not good.

the thing is that some data of rmw_init_options_t needs to be updated via rcl_init before rmw_init, and a part of those is enclave which needs to be copied to rmw_init_options_t.

Move the strndup out of rcl_init and into rmw_init, and leave the free in the RMW.

i am also inclined to this option, it is important to keep the clear responsibility for memory management for maintenance.

@clalancette rmw_init requires rmw_init_options_t, so probably we need to have rmw_init(free)_options_enclave to allocate(deallocate) the memory in rmw implementation. what do you think?

i can patch this if we are good to go. i can also give it a shot to rmw_microxrcedds, as far as i see static memory management is done only on rmw_init_options_t structure, i think we need to call deallocator before returning that memory slot to the static buffer. @pablogs9 @gavanderhoorn am I understanding correctly? actually this is 1st time to see this code... 😅 )

@fujitatomoya that's it. I guess that checking if init_options->enclave != nullptr and deallocating it using the default allocator would work.

When this responsibility is moved to rmw implementations, the correct option is not to allocate nor deallocate this as rmw_microxrcedds does not handle SROS2 security features.

@pablogs9 can you also close this since #318 is merged.

@fujitatomoya thank you, I appreciate it!

Are these changes expected to be backported and added to future humble and jazzy releases? Or is this change likely just for future ROS2 distributions?

No i do not think so. rolling fix adds the new interfaces, so unlikely backported to already released distro. let me see how jazzy and humble can be fixed.