Open fujitatomoya opened 1 year ago
this can also happen with humble branch as well.
CC: @iuhilnehc-ynos @Barry-Xu-2018
Actually, I noticed this issue a long time ago while adding the zero_allocate.
The deallocate
just deallocate buffer size with 1. I don't know how to fix this issue elegantly.
Any suggestion?
How about incorporating an additional size field at the beginning of the buffer to store the size value during buffer allocation?
(NOTE: It results in a significant waste of memory when there are numerous small buffer allocations.)
According to cppreference the ending parameter of both allocate
and deallocate
is a size_type n
where n is:
the number of objects to allocate storage for
So is there anyway to extract the size of the typename T given in retyped_deallocate
like
std::allocator_traits<Alloc>::deallocate(*typed_allocator, typed_ptr, sizeof(T));
or a some way to find the type of the object that was allocated, or is that beyond the reach of the function.
What if the type is char
and then rcl
uses the allocator to allocate a buffer with count
?
Even if we can add an explicit specialization template get_rcl_allocator
for char
to use rcl_get_default_allocator(), which seems like a workaround, what if using get_rcl_allocator<rcl_node_t>
in the future, and then use the allocator to allocate a buffer with count(2)
?
It seems that using sizeof(T)
can't fix this issue.
I stumbled on this issue, because I encountered problems when I wanted to use ROS2 >= humble with jemalloc. (Apparently I am not the first one: ROS 2 migration stories: The struggles of moving to Humble)
I think std::allocator_traits<Alloc>::deallocate(*typed_allocator, typed_ptr, sizeof(T));
won't work because n
is the number of objects of type T
to deallocate and not the number of bytes.
I also think that https://github.com/ros2/rclcpp/blob/978439191fdb3924af900a506dd35b68f8da725c/rclcpp/include/rclcpp/allocator/allocator_common.hpp#L67 only works because the default implementation of operator delete(void* ptr, std::size_t size)
just calls operator delete(void* ptr)
and ignores the size argument. Jemalloc instead explicitly implements a sized delete, which leads to memory errors when used together with rclcpp, because the size argument does not match the previously allocated size. So I wonder if there is a safe way to create an rcl_allocator_t
struct from an STL allocator, because the STL allocator requires a size argument for deallocate
.
I'm assuming that the C++ standard writers had a good reason to make the number of elements a parameter. Therefore, I would suggest to address this by adding a corresponding method which takes this as an argument. Further, we may want to consider deprecating the existing version of the function that does not have this argument.
Bug report
Required Info:
rolling
Steps to reproduce issue
Expected behavior
No ASAN report generated.
Actual behavior
The following ASAN report generated.
AddressSanitizer: new-delete-type-mismatch
```bash root@tomoyafujita:~/ros2_ws/colcon_ws# ros2 run demo_nodes_cpp talker ================================================================= ==726931==ERROR: AddressSanitizer: new-delete-type-mismatch on 0x60600006c0e0 in thread T0: object passed to delete has wrong type: size of the allocated type: 64 bytes; size of the deallocated type: 1 bytes. #0 0x7f7b840bd22f in operator delete(void*, unsigned long) ../../../../src/libsanitizer/asan/asan_new_delete.cpp:172 #1 0x7f7b82772587 in rcutils_string_map_fini /root/ros2_ws/colcon_ws/src/ros2/rcutils/src/string_map.c:105 #2 0x7f7b8354dfb0 in rcl_resolve_name /root/ros2_ws/colcon_ws/src/ros2/rcl/rcl/src/rcl/node_resolve_name.c:110 #3 0x7f7b8354e751 in rcl_node_resolve_name /root/ros2_ws/colcon_ws/src/ros2/rcl/rcl/src/rcl/node_resolve_name.c:152 #4 0x7f7b835483e5 in rcl_publisher_init /root/ros2_ws/colcon_ws/src/ros2/rcl/rcl/src/rcl/publisher.c:83 #5 0x7f7b83c0bc16 in rclcpp::PublisherBase::PublisherBase(rclcpp::node_interfaces::NodeBaseInterface*, std::__cxx11::basic_string