search

ros / class_loader

ROS-independent library for dynamic class (i.e. plugin) introspection and loading from runtime libraries
http://www.ros.org/wiki/class_loader
34 stars 95 forks source link

Remove raw pointer interface #197

Open bpwilcox opened 2 years ago

bpwilcox commented 2 years ago

There have been a few issues already referencing the known memory leak in this code with the metaobjects. https://github.com/ros/class_loader/issues/131 https://github.com/ros/pluginlib/issues/126

In https://github.com/ros/class_loader/pull/186 to potentially resolve these memory leaks, @hidmic mentioned a goal to remove the raw pointer interface from the existing code. https://github.com/ros/class_loader/pull/186#pullrequestreview-794485117. As humble will soon be released, I wanted to create this ticket to formally track this effort.

Was there a reason or limitation for not using smart pointers (e.g. a smart pointer for AbstractMetaObjectBase in the FactoryMap / graveyard MetaObjectVector)?

tylerjw commented 2 years ago

It would be really nice if this was fixed and a CI job added to test class_loader (and pluginlib) with ASAN to defend it against this coming back. @v4hn commented here about it: https://github.com/ros/class_loader/issues/131#issuecomment-849574914

Without a tool like ASAN and tests, any software like this that does manual memory management is doomed to have memory errors unless the authors of the software are prefect. Here is an example of a memory error in rclcpp I also discovered today by trying to use asan in my project's CI that depends on these various tools in the ros ecosystem: https://github.com/ros2/rclcpp/issues/1948

To error is human, to write tests and use sanitizers is to have compassion for your users.

v-lopez commented 2 years ago

+1, we try to have asan on our code, and are limited to having to deal with these leaks.