Fix ASAN / valgrind warnings due to C / C++ allocator mismatch#3054
Fix ASAN / valgrind warnings due to C / C++ allocator mismatch#3054jmachowinski wants to merge 3 commits intoros2:rollingfrom
Conversation
As explained in ros2#1254, there's conceptually no way to implement RCL allocators in terms of C++ allocators. In order to fix this behavior, we have to delete the generic version of get_rcl_allocator. Since some ROS code depends on this, we need to allow users to write their own version of get_rcl_allocator for allocators that support the C-style interface (most do). So this CL changes get_rcl_allocator from a template function into a family of (potentially templated) functions, which allows users to add their own overloads and rely on the "most specialized" mechanism for function specialization to select the right one. See http://www.gotw.ca/publications/mill17.htm for details. This also allows us to return get_rcl_default_allocator for all specializations of std::allocator (previously, only for std::allocator), which will already fix ros2#1254 for pretty much all clients. I'll continue to work on deleting the generic version, though, to make sure that nobody is accidentally bitten by it. I've tried to test this by doing a full ROS compilation following the Dockerfile of the source Docker image, and all packages compile. Signed-off-by: Steve Wolter <swolter@google.com> Addressed code style test failures. Signed-off-by: Steve Wolter <swolter@google.com> Update comments. Incidentally, this also reruns the flaky test suite, which seems to be using the real DDS middleware and to be prone to cross-talk. Signed-off-by: Steve Wolter <swolter@google.com> Also update recently added tests. Signed-off-by: Steve Wolter <swolter@google.com> Added reference to bug number. Signed-off-by: Steve Wolter <swolter@google.com> Extend allocator lifetime in options. Signed-off-by: Steve Wolter <swolter@google.com> Drop the generic version of get_rcl_allocator. This allows us to simplify a bunch of code as well. Signed-off-by: Steve Wolter <swolter@google.com> Update rclcpp/include/rclcpp/allocator/allocator_common.hpp Co-authored-by: William Woodall <william+github@osrfoundation.org> Signed-off-by: Steve Wolter <swolter@google.com> Revert accidental deletion of allocator creation. Signed-off-by: Steve Wolter <swolter@google.com>
|
Pulls: #3054 |
1d805dd to
d36dae6
Compare
|
Pulls: #3054 |
fujitatomoya
left a comment
There was a problem hiding this comment.
lgtm with green CI.
really hope we can merge it this time 🤞
|
Just a note that this does break API. |
Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com>
d36dae6 to
aab8c9e
Compare
|
Pulls: #3054 |
Description
rclcpp passes down C++ style allocators down to rcl, that then uses them.
Due to the missing / wrong size on the deallocation, all the memory checkers
complain about it in newer ubuntu versions (>= 24.04).
This PR is a rebase / rework of #2046
Did you use Generative AI?
No