Dynamic Subscription (REP-2011 subset): BONUS: Allocator Refactor

[methylDragon]

Description

No long detailed description this time, unfortunately...

This issue describes and lists the relevant PRs that build on top of the PRs in #1374 with non-essential, bonus changes for ease of review and decoupling.

The base PRs should be prioritized.

The PRs in #1374 assumed that most of the objects being introduced can only live on the heap, but this wave of PRs allows those objects to be created on the stack and then initialized.

The changes here are namely refactors across the entire stack for function signatures to take in allocators, and splitting of create() functions into init() and create() (likewise for destroy into fini() and destroy().)

That is, splitting the steps of:

• Allocating an outer struct (create/new)
• Allocating and initializing the internal members of that struct (init)

And similarly for destruction:

• Deallocating the outer struct (delete/destroy)
• Deallocating and finalizing the internal members of that struct (fini)

NOTE

These changes will be occurring mostly in the rmw and rosidl layers

Relevant PRs

• Add convenience error handling macros rcutils#421
• Dynamic Subscription (BONUS: Allocators): rosidl rosidl#737
• Dynamic Subscription (BONUS: Allocators): rosidl_dynamic_typesupport rosidl_dynamic_typesupport#2
• Dynamic Subscription (BONUS: Allocators): rosidl_dynamic_typesupport_fastrtps rosidl_dynamic_typesupport_fastrtps#3
• Dynamic Subscription (BONUS: Allocators): rmw rmw#353
• Dynamic Subscription (BONUS: Allocators): rclcpp rclcpp#2160
  • Runtime Interface Reflection: rclcpp (Rebased) rclcpp#2176
• Dynamic Subscription (BONUS: Allocators): rcl rcl#1057
• RMWs (must be merged together)
  • Dynamic Subscription (BONUS: Allocators): rmw_implementation rmw_implementation#219
  • Dynamic Subscription (BONUS: Allocators): rmw_connextdds rmw_connextdds#115
  • Dynamic Subscription (BONUS: Allocators): rmw_cyclonedds rmw_cyclonedds#451
  • Dynamic Subscription (BONUS: Allocators): rmw_fastrtps rmw_fastrtps#687

TODOs

I might not get all of them, but wow am I going to try:
(Note: Edits in each lower level struct will need changes propagated upwards, but the higher level structs are what users are more likely to interact with.)

• Message Type Support
  • rosidl_message_type_support_t
  • rosidl_dynamic_message_type_support_impl_t
• Dynamic Type Support
  • rosidl_dynamic_typesupport_serialization_support_t
    • getting this in means all dynamic typesupport types will have access to a user-configured allocator via their composed serialization supports (but doesn't yet unlock the ability to have independent allocators from the serialization support)
  • rosidl_dynamic_typesupport_dynamic_type_t
  • rosidl_dynamic_typesupport_dynamic_type_builder_t
  • rosidl_dynamic_typesupport_dynamic_data_t
• Dynamic Type Support Impls (Do these even make sense to take allocators? 🤔 I don't think I will support them)
  • rosidl_dynamic_typesupport_serialization_support_impl_t
  • rosidl_dynamic_typesupport_dynamic_type_impl_t
  • rosidl_dynamic_typesupport_dynamic_type_builder_impl_t
  • rosidl_dynamic_typesupport_dynamic_data_impl_t
• Type Description Utils
  • This is a whole bunch of stuff... I might not get to this
• rclcpp (I'm not even going to think about this one yet)
  • But for my reference... https://github.com/ros2/rclcpp/blob/rolling/rclcpp/include/rclcpp/subscription_options.hpp

[methylDragon]

• Linux
• Linux-aarch64
• Windows

[methylDragon]

• Linux
• Linux-aarch64
• Windows

[methylDragon]

I'm gonna call this feature done! It's running fine and there aren't any memory/runtime issues that I can see.

⚠️ : It's paramount to merge this because this significantly cleans up the memory management (pointers are replaced with values wherever possible), and also allows for users to use custom allocators with every struct in the dynamic types support stack.

Valgrind Logs of the Cleanup Come Up COMPLETELY CLEAN (except for pre-existing bugs in FastRTPS) (well.. I guess and some leaks)

The Valgrind Cleanup Logs

^C[INFO] [1681132282.322891325] [rclcpp]: signal_handler(signum=2)
==200582== Invalid read of size 1
==200582==    at 0x4FE54D4: eprosima::fastrtps::types::DynamicType::has_children() const (DynamicType.cpp:464)
==200582==    by 0x4FCB4C6: eprosima::fastrtps::types::DynamicData::clean_members() (DynamicData.cpp:691)
==200582==    by 0x4FCB61E: eprosima::fastrtps::types::DynamicData::clean() (DynamicData.cpp:636)
==200582==    by 0x4FCB6C1: eprosima::fastrtps::types::DynamicData::~DynamicData() (DynamicData.cpp:142)
==200582==    by 0x4FE2609: eprosima::fastrtps::types::DynamicDataFactory::delete_data(eprosima::fastrtps::types::DynamicData*) (DynamicDataFactory.cpp:215)
==200582==    by 0x777EAF5: fastrtps__dynamic_data_fini(rosidl_dynamic_typesupport_serialization_support_impl_s*, rosidl_dynamic_typesupport_dynamic_data_impl_s*) (fastrtps_dynamic_data.cpp:299)
==200582==    by 0x49D5B74: rclcpp::dynamic_typesupport::DynamicMessage::~DynamicMessage() (dynamic_message.cpp:174)
==200582==    by 0x49DE09E: _M_release (shared_ptr_base.h:168)
==200582==    by 0x49DE09E: ~__shared_count (shared_ptr_base.h:705)
==200582==    by 0x49DE09E: ~__shared_ptr (shared_ptr_base.h:1154)
==200582==    by 0x49DE09E: reset (shared_ptr_base.h:1272)
==200582==    by 0x49DE09E: rclcpp::dynamic_typesupport::DynamicMessageTypeSupport::~DynamicMessageTypeSupport() (dynamic_message_type_support.cpp:228)
==200582==    by 0x1126D1: std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release() (shared_ptr_base.h:168)
==200582==    by 0x11078E: ~__shared_count (shared_ptr_base.h:705)
==200582==    by 0x11078E: ~__shared_ptr (shared_ptr_base.h:1154)
==200582==    by 0x11078E: ~shared_ptr (shared_ptr.h:122)
==200582==    by 0x11078E: main (dynamic_sub_serialized.cpp:117)
==200582==  Address 0xccb36f0 is 144 bytes inside a block of size 152 free'd
==200582==    at 0x484F8AF: operator delete(void*) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==200582==    by 0x4FF3705: eprosima::fastrtps::types::DynamicTypeBuilderFactory::delete_type(eprosima::fastrtps::types::DynamicType*) (DynamicTypeBuilderFactory.cpp:823)
==200582==    by 0x7782FAA: fastrtps__dynamic_type_fini(rosidl_dynamic_typesupport_serialization_support_impl_s*, rosidl_dynamic_typesupport_dynamic_type_impl_s*) (fastrtps_dynamic_type.cpp:235)
==200582==    by 0x49D99E1: rclcpp::dynamic_typesupport::DynamicMessageType::~DynamicMessageType() (dynamic_message_type.cpp:109)
==200582==    by 0x49DE031: _M_release (shared_ptr_base.h:168)
==200582==    by 0x49DE031: ~__shared_count (shared_ptr_base.h:705)
==200582==    by 0x49DE031: ~__shared_ptr (shared_ptr_base.h:1154)
==200582==    by 0x49DE031: reset (shared_ptr_base.h:1272)
==200582==    by 0x49DE031: rclcpp::dynamic_typesupport::DynamicMessageTypeSupport::~DynamicMessageTypeSupport() (dynamic_message_type_support.cpp:227)
==200582==    by 0x1126D1: std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release() (shared_ptr_base.h:168)
==200582==    by 0x11078E: ~__shared_count (shared_ptr_base.h:705)
==200582==    by 0x11078E: ~__shared_ptr (shared_ptr_base.h:1154)
==200582==    by 0x11078E: ~shared_ptr (shared_ptr.h:122)
==200582==    by 0x11078E: main (dynamic_sub_serialized.cpp:117)
==200582==  Block was alloc'd at
==200582==    at 0x484D013: operator new(unsigned long) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==200582==    by 0x4FF521C: eprosima::fastrtps::types::DynamicTypeBuilderFactory::create_type(eprosima::fastrtps::types::DynamicTypeBuilder const*) (DynamicTypeBuilderFactory.cpp:180)
==200582==    by 0x4FEDDBD: eprosima::fastrtps::types::DynamicTypeBuilder::build() (DynamicTypeBuilder.cpp:302)
==200582==    by 0x7783428: fastrtps__dynamic_type_init_from_dynamic_type_builder(rosidl_dynamic_typesupport_serialization_support_impl_s*, rosidl_dynamic_typesupport_dynamic_type_builder_impl_s*, rcutils_allocator_s*, rosidl_dynamic_typesupport_dynamic_type_impl_s*) (fastrtps_dynamic_type.cpp:174)
==200582==    by 0x54BF952: rosidl_dynamic_typesupport_dynamic_type_init_from_dynamic_type_builder (dynamic_type.c:902)
==200582==    by 0x54C4A35: rosidl_dynamic_typesupport_dynamic_type_init_from_description (dynamic_type.c:945)
==200582==    by 0x54C4EE0: rosidl_dynamic_message_type_support_handle_impl_init (dynamic_message_type_support_struct.c:192)
==200582==    by 0x54C52CC: rosidl_dynamic_message_type_support_handle_init (dynamic_message_type_support_struct.c:67)
==200582==    by 0x547CC87: rcl_dynamic_message_type_support_handle_init (dynamic_message_type_support.c:91)
==200582==    by 0x49DEAEA: rclcpp::dynamic_typesupport::DynamicMessageTypeSupport::DynamicMessageTypeSupport(rosidl_runtime_c__type_description__TypeDescription const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, rcutils_allocator_s) (dynamic_message_type_support.cpp:60)
==200582==    by 0x1128D8: construct<rclcpp::dynamic_typesupport::DynamicMessageTypeSupport, rosidl_runtime_c__type_description__TypeDescription&> (new_allocator.h:162)
==200582==    by 0x1128D8: construct<rclcpp::dynamic_typesupport::DynamicMessageTypeSupport, rosidl_runtime_c__type_description__TypeDescription&> (alloc_traits.h:516)
==200582==    by 0x1128D8: _Sp_counted_ptr_inplace<rosidl_runtime_c__type_description__TypeDescription&> (shared_ptr_base.h:519)
==200582==    by 0x1128D8: __shared_count<rclcpp::dynamic_typesupport::DynamicMessageTypeSupport, std::allocator<rclcpp::dynamic_typesupport::DynamicMessageTypeSupport>, rosidl_runtime_c__type_description__TypeDescription&> (shared_ptr_base.h:650)
==200582==    by 0x1128D8: std::__shared_ptr<rclcpp::dynamic_typesupport::DynamicMessageTypeSupport, (__gnu_cxx::_Lock_policy)2>::__shared_ptr<std::allocator<rclcpp::dynamic_typesupport::DynamicMessageTypeSupport>, rosidl_runtime_c__type_description__TypeDescription&>(std::_Sp_alloc_shared_tag<std::allocator<rclcpp::dynamic_typesupport::DynamicMessageTypeSupport> >, rosidl_runtime_c__type_description__TypeDescription&) (shared_ptr_base.h:1342)
==200582==    by 0x110373: shared_ptr<std::allocator<rclcpp::dynamic_typesupport::DynamicMessageTypeSupport>, rosidl_runtime_c__type_description__TypeDescription&> (shared_ptr.h:409)
==200582==    by 0x110373: allocate_shared<rclcpp::dynamic_typesupport::DynamicMessageTypeSupport, std::allocator<rclcpp::dynamic_typesupport::DynamicMessageTypeSupport>, rosidl_runtime_c__type_description__TypeDescription&> (shared_ptr.h:863)
==200582==    by 0x110373: make_shared<rclcpp::dynamic_typesupport::DynamicMessageTypeSupport, rosidl_runtime_c__type_description__TypeDescription&> (shared_ptr.h:879)
==200582==    by 0x110373: make_shared<rosidl_runtime_c__type_description__TypeDescription&> (dynamic_message_type_support.hpp:62)
==200582==    by 0x110373: main (dynamic_sub_serialized.cpp:99)
==200582== 
==200582== Invalid read of size 1
==200582==    at 0x4FCB407: eprosima::fastrtps::types::DynamicData::get_kind() const (DynamicData.cpp:460)
==200582==    by 0x4FCB547: eprosima::fastrtps::types::DynamicData::clean_members() (DynamicData.cpp:700)
==200582==    by 0x4FCB61E: eprosima::fastrtps::types::DynamicData::clean() (DynamicData.cpp:636)
==200582==    by 0x4FCB6C1: eprosima::fastrtps::types::DynamicData::~DynamicData() (DynamicData.cpp:142)
==200582==    by 0x4FE2609: eprosima::fastrtps::types::DynamicDataFactory::delete_data(eprosima::fastrtps::types::DynamicData*) (DynamicDataFactory.cpp:215)
==200582==    by 0x777EAF5: fastrtps__dynamic_data_fini(rosidl_dynamic_typesupport_serialization_support_impl_s*, rosidl_dynamic_typesupport_dynamic_data_impl_s*) (fastrtps_dynamic_data.cpp:299)
==200582==    by 0x49D5B74: rclcpp::dynamic_typesupport::DynamicMessage::~DynamicMessage() (dynamic_message.cpp:174)
==200582==    by 0x49DE09E: _M_release (shared_ptr_base.h:168)
==200582==    by 0x49DE09E: ~__shared_count (shared_ptr_base.h:705)
==200582==    by 0x49DE09E: ~__shared_ptr (shared_ptr_base.h:1154)
==200582==    by 0x49DE09E: reset (shared_ptr_base.h:1272)
==200582==    by 0x49DE09E: rclcpp::dynamic_typesupport::DynamicMessageTypeSupport::~DynamicMessageTypeSupport() (dynamic_message_type_support.cpp:228)
==200582==    by 0x1126D1: std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release() (shared_ptr_base.h:168)
==200582==    by 0x11078E: ~__shared_count (shared_ptr_base.h:705)
==200582==    by 0x11078E: ~__shared_ptr (shared_ptr_base.h:1154)
==200582==    by 0x11078E: ~shared_ptr (shared_ptr.h:122)
==200582==    by 0x11078E: main (dynamic_sub_serialized.cpp:117)
==200582==  Address 0xccb36f0 is 144 bytes inside a block of size 152 free'd
==200582==    at 0x484F8AF: operator delete(void*) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==200582==    by 0x4FF3705: eprosima::fastrtps::types::DynamicTypeBuilderFactory::delete_type(eprosima::fastrtps::types::DynamicType*) (DynamicTypeBuilderFactory.cpp:823)
==200582==    by 0x7782FAA: fastrtps__dynamic_type_fini(rosidl_dynamic_typesupport_serialization_support_impl_s*, rosidl_dynamic_typesupport_dynamic_type_impl_s*) (fastrtps_dynamic_type.cpp:235)
==200582==    by 0x49D99E1: rclcpp::dynamic_typesupport::DynamicMessageType::~DynamicMessageType() (dynamic_message_type.cpp:109)
==200582==    by 0x49DE031: _M_release (shared_ptr_base.h:168)
==200582==    by 0x49DE031: ~__shared_count (shared_ptr_base.h:705)
==200582==    by 0x49DE031: ~__shared_ptr (shared_ptr_base.h:1154)
==200582==    by 0x49DE031: reset (shared_ptr_base.h:1272)
==200582==    by 0x49DE031: rclcpp::dynamic_typesupport::DynamicMessageTypeSupport::~DynamicMessageTypeSupport() (dynamic_message_type_support.cpp:227)
==200582==    by 0x1126D1: std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release() (shared_ptr_base.h:168)
==200582==    by 0x11078E: ~__shared_count (shared_ptr_base.h:705)
==200582==    by 0x11078E: ~__shared_ptr (shared_ptr_base.h:1154)
==200582==    by 0x11078E: ~shared_ptr (shared_ptr.h:122)
==200582==    by 0x11078E: main (dynamic_sub_serialized.cpp:117)
==200582==  Block was alloc'd at
==200582==    at 0x484D013: operator new(unsigned long) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==200582==    by 0x4FF521C: eprosima::fastrtps::types::DynamicTypeBuilderFactory::create_type(eprosima::fastrtps::types::DynamicTypeBuilder const*) (DynamicTypeBuilderFactory.cpp:180)
==200582==    by 0x4FEDDBD: eprosima::fastrtps::types::DynamicTypeBuilder::build() (DynamicTypeBuilder.cpp:302)
==200582==    by 0x7783428: fastrtps__dynamic_type_init_from_dynamic_type_builder(rosidl_dynamic_typesupport_serialization_support_impl_s*, rosidl_dynamic_typesupport_dynamic_type_builder_impl_s*, rcutils_allocator_s*, rosidl_dynamic_typesupport_dynamic_type_impl_s*) (fastrtps_dynamic_type.cpp:174)
==200582==    by 0x54BF952: rosidl_dynamic_typesupport_dynamic_type_init_from_dynamic_type_builder (dynamic_type.c:902)
==200582==    by 0x54C4A35: rosidl_dynamic_typesupport_dynamic_type_init_from_description (dynamic_type.c:945)
==200582==    by 0x54C4EE0: rosidl_dynamic_message_type_support_handle_impl_init (dynamic_message_type_support_struct.c:192)
==200582==    by 0x54C52CC: rosidl_dynamic_message_type_support_handle_init (dynamic_message_type_support_struct.c:67)
==200582==    by 0x547CC87: rcl_dynamic_message_type_support_handle_init (dynamic_message_type_support.c:91)
==200582==    by 0x49DEAEA: rclcpp::dynamic_typesupport::DynamicMessageTypeSupport::DynamicMessageTypeSupport(rosidl_runtime_c__type_description__TypeDescription const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, rcutils_allocator_s) (dynamic_message_type_support.cpp:60)
==200582==    by 0x1128D8: construct<rclcpp::dynamic_typesupport::DynamicMessageTypeSupport, rosidl_runtime_c__type_description__TypeDescription&> (new_allocator.h:162)
==200582==    by 0x1128D8: construct<rclcpp::dynamic_typesupport::DynamicMessageTypeSupport, rosidl_runtime_c__type_description__TypeDescription&> (alloc_traits.h:516)
==200582==    by 0x1128D8: _Sp_counted_ptr_inplace<rosidl_runtime_c__type_description__TypeDescription&> (shared_ptr_base.h:519)
==200582==    by 0x1128D8: __shared_count<rclcpp::dynamic_typesupport::DynamicMessageTypeSupport, std::allocator<rclcpp::dynamic_typesupport::DynamicMessageTypeSupport>, rosidl_runtime_c__type_description__TypeDescription&> (shared_ptr_base.h:650)
==200582==    by 0x1128D8: std::__shared_ptr<rclcpp::dynamic_typesupport::DynamicMessageTypeSupport, (__gnu_cxx::_Lock_policy)2>::__shared_ptr<std::allocator<rclcpp::dynamic_typesupport::DynamicMessageTypeSupport>, rosidl_runtime_c__type_description__TypeDescription&>(std::_Sp_alloc_shared_tag<std::allocator<rclcpp::dynamic_typesupport::DynamicMessageTypeSupport> >, rosidl_runtime_c__type_description__TypeDescription&) (shared_ptr_base.h:1342)
==200582==    by 0x110373: shared_ptr<std::allocator<rclcpp::dynamic_typesupport::DynamicMessageTypeSupport>, rosidl_runtime_c__type_description__TypeDescription&> (shared_ptr.h:409)
==200582==    by 0x110373: allocate_shared<rclcpp::dynamic_typesupport::DynamicMessageTypeSupport, std::allocator<rclcpp::dynamic_typesupport::DynamicMessageTypeSupport>, rosidl_runtime_c__type_description__TypeDescription&> (shared_ptr.h:863)
==200582==    by 0x110373: make_shared<rclcpp::dynamic_typesupport::DynamicMessageTypeSupport, rosidl_runtime_c__type_description__TypeDescription&> (shared_ptr.h:879)
==200582==    by 0x110373: make_shared<rosidl_runtime_c__type_description__TypeDescription&> (dynamic_message_type_support.hpp:62)
==200582==    by 0x110373: main (dynamic_sub_serialized.cpp:99)
==200582== 
2023-04-10 06:11:22.543 [DYN_TYPES Error] Error deleting DynamicData. It isn't registered in the factory -> Function delete_data
==200582== 
==200582== HEAP SUMMARY:
==200582==     in use at exit: 49,918 bytes in 158 blocks
==200582==   total heap usage: 29,479 allocs, 29,321 frees, 6,362,026 bytes allocated
==200582== 
==200582== LEAK SUMMARY:
==200582==    definitely lost: 268 bytes in 6 blocks
==200582==    indirectly lost: 64 bytes in 3 blocks
==200582==      possibly lost: 768 bytes in 2 blocks
==200582==    still reachable: 48,818 bytes in 147 blocks
==200582==         suppressed: 0 bytes in 0 blocks
==200582== Rerun with --leak-check=full to see details of leaked memory

• Linux (The agent disconnected, the failure isn't due to the PRs)
• Linux-aarch64
• Windows
• Windows Debug
• RHEL

(I'm expecting one lint failure that I fixed post-hoc)

[methylDragon]

Run 2 for posterity: (these are unstable only because of uncrustify!!)

• Linux
• Linux-aarch64
• Windows
• Windows Debug
• RHEL

[methylDragon]

• Linux
• Linux-aarch64
• Windows
• Windows Debug
• RHEL

[methylDragon]

• Linux
• Linux-aarch64
• Windows

---

• Linux
• Linux-aarch64
• Windows
• Windows Debug
• RHEL

[clalancette]

After discussion, the future work here is as follows:

1. Get Dynamic Subscription (BONUS: Allocators): rclcpp rclcpp#2160 reviewed and merged into Rolling.
2. Get Dynamic Subscription (BONUS: Allocators): rclcpp rclcpp#2160 backported to Iron for a patch release
3. Implement dynamic type support for Connext (probably needs a new rosidl_dynamic_typesupport_connext package).
4. Update rmw_connextdds to take advantage of the above.
5. Implement dynamic type support for CycloneDDS (probably needs a new rosidl_dynamic_typesupport_cyclonedds package).
6. Update rmw_cyclonedds to take advantage of the above.
7. Documentation and examples for the feature.

What I'll suggest is that after we get the first two items done, we close out this issue and open a new one to track the remaining work.