norlab-ulaval / libpointmatcher

An Iterative Closest Point (ICP) library for 2D and 3D mapping in Robotics
BSD 3-Clause "New" or "Revised" License
1.6k stars 544 forks source link

DataPointsFilters, OutlierFilters, Transformations, and TransformationCheckers structs inherit from a class without virtual destructor #423

Open YoshuaNava opened 3 years ago

YoshuaNava commented 3 years ago

The structs mentioned in the description inherit directly from std::vector, an STL container. For example:

https://github.com/ethz-asl/libpointmatcher/blob/master/pointmatcher/PointMatcher.h#L422

The above can lead to undefined behavior, as the STL containers don't have virtual destructors. Therefore, any collection of polymorphic objects from those types could have issues getting destroyed.

References:

YoshuaNava commented 3 years ago

An example of how this could impact usage of the library is when people inherit from one of the containers cited above, as DataPointsFilters/Gestalt. This filter is stateless (saves no state, only parameters): https://github.com/ethz-asl/libpointmatcher/blob/master/pointmatcher/DataPointsFilters/Gestalt.h#L94

When the filter is created, used and tore down, it is undefined how the parameters are destructed. If we were to implement a filter that uses history, this could result in a memory leak in the worst case.

pomerlef commented 3 years ago

Interesting reading this morning. What would be your solution here?