use unique ptr and remove unuseful container

[iuhilnehc-ynos]

related to ros2/rcl#1009 (comment)

Signed-off-by: Chen Lihui lihui.chen@sony.com

[alsora]

Would you mind providing more details about the lifespan of the loaded libraries?
Are the libraries unloaded when the unique pointer goes out of scope?

[iuhilnehc-ynos]

@alsora

Thanks for asking questions.
I am not sure if the following backtrace information can answer that.

(gdb) bt
#0  class_loader::impl::loadLibrary (library_path="libtopics_library.so", loader=0x5555555fca20)
    at /home/chenlh/Projects/ROS2/ros2-master/src/ros/class_loader/src/class_loader_core.cpp:432
#1  0x00007ffff71ab25f in class_loader::ClassLoader::loadLibrary (this=0x5555555fca20)
    at /home/chenlh/Projects/ROS2/ros2-master/src/ros/class_loader/src/class_loader.cpp:106
#2  0x00007ffff71ab091 in class_loader::ClassLoader::ClassLoader (this=0x5555555fca20, library_path="libtopics_library.so", ondemand_load_unload=false)
    at /home/chenlh/Projects/ROS2/ros2-master/src/ros/class_loader/src/class_loader.cpp:65
#3  0x00005555555683ca in std::make_unique<class_loader::ClassLoader, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&>
    () at /usr/include/c++/9/bits/unique_ptr.h:857
#4  0x0000555555565f65 in main (argc=1, argv=0x7fffffffc5d8)
    at /home/chenlh/Projects/ROS2/ros2-master/build/demo_nodes_cpp/rclcpp_components/node_main_talker.cpp:39

(gdb) bt
#0  class_loader::impl::unloadLibrary (library_path="libtopics_library.so", loader=0x5555555fca20)
    at /home/chenlh/Projects/ROS2/ros2-master/src/ros/class_loader/src/class_loader_core.cpp:539
#1  0x00007ffff71ab3b4 in class_loader::ClassLoader::unloadLibraryInternal (this=0x5555555fca20, lock_plugin_ref_count=true)
    at /home/chenlh/Projects/ROS2/ros2-master/src/ros/class_loader/src/class_loader.cpp:136
#2  0x00007ffff71ab2ef in class_loader::ClassLoader::unloadLibrary (this=0x5555555fca20)
    at /home/chenlh/Projects/ROS2/ros2-master/src/ros/class_loader/src/class_loader.cpp:115
#3  0x00007ffff71ab115 in class_loader::ClassLoader::~ClassLoader (this=0x5555555fca20, __in_chrg=<optimized out>)
    at /home/chenlh/Projects/ROS2/ros2-master/src/ros/class_loader/src/class_loader.cpp:75
#4  0x00007ffff71ab144 in class_loader::ClassLoader::~ClassLoader (this=0x5555555fca20, __in_chrg=<optimized out>)
    at /home/chenlh/Projects/ROS2/ros2-master/src/ros/class_loader/src/class_loader.cpp:76
#5  0x00005555555691c0 in std::default_delete<class_loader::ClassLoader>::operator() (this=0x7fffffffb638, __ptr=0x5555555fca20)
    at /usr/include/c++/9/bits/unique_ptr.h:81
#6  0x000055555556845e in std::unique_ptr<class_loader::ClassLoader, std::default_delete<class_loader::ClassLoader> >::~unique_ptr (
    this=0x7fffffffb638, __in_chrg=<optimized out>) at /usr/include/c++/9/bits/unique_ptr.h:292
#7  0x00005555555664b9 in main (argc=1, argv=0x7fffffffc5d8)
    at /home/chenlh/Projects/ROS2/ros2-master/build/demo_nodes_cpp/rclcpp_components/node_main_talker.cpp:39

The key to fix ros2/rcl#1009 is to call class_loader::ClassLoader::unloadLibrary in the ClassLoader destructor to use purgeGraveyardOfMetaobjects to destroy MetaObject which created by the macro before.

[fujitatomoya]

lgtm

[fujitatomoya]

this does not depend on ros/class_loader#200, but related.

[alsora]

@iuhilnehc-ynos thank you.
From the back trace it looks like the libraries are unloaded when the unique pointers go out of scope.
This happens right after a node is created.

I'm a little bit concerned about this.
Usually when dealing with dynamically loaded libraries it is best practice to keep the library in scope until objects are using it.
This is important especially for architectures using linkers with lazy binding (I.e. Symbols are not resolved when you load a library but rather when the first time you use them).

I think we should keep the libraries loaded until we have destroyed the nodes.

[iuhilnehc-ynos]

I think we should keep the libraries loaded until we have destroyed the nodes.

rclcpp/rclcpp_components/src/node_main.cpp.in

Line 67 in 3233f03

node_wrappers.clear();

can destroy the nodes before unloading the libraries.

[fujitatomoya]

CI:

• Linux
• Linux-aarch64
• Windows

[iuhilnehc-ynos]

@ros-pull-request-builder retest this please

• Windows

[iuhilnehc-ynos]

Error happened in the CI as following
14:08:19 CVTRES : fatal error CVT1107: 'C:\Program Files\rti_connext_dds-6.0.1\lib\x64Win64VS2017\nddscored.lib' is corrupt

Give it another shot,

• Windows

[fujitatomoya]

@Mergifyio backport humble galactic foxy

[mergify]

backport humble galactic foxy

✅ Backports have been created

• #2014 use unique ptr and remove unuseful container (backport #2013) has been created for branch humble
• #2015 use unique ptr and remove unuseful container (backport #2013) has been created for branch galactic
• #2016 use unique ptr and remove unuseful container (backport #2013) has been created for branch foxy

[fujitatomoya]

@iuhilnehc-ynos can you take care of the backpors for foxy/galactic/humble?