allocator_common.hpp overallocates 100x memory & invokes UB on dealloc #1254
Description
rclcpp/include/rclcpp/allocator/allocator_common.hpp has two severe correctness issues in memory allocation:
- retyped_allocate takes the "size" parameter (which is size in bytes as defined by rcl_allocator_t) and passes it to it's allocator's allocate function (which takes number of elements as its parameter). So when asked to allocate N bytes, it actually allocates N*sizeof(T) bytes, where T is the type of the allocator. It's called by subscribers and publishers with a typed allocator of type "Message", which is easily ~100 bytes, so it's overallocating memory by a factor of ~100.
- retyped_deallocate passes 1 to the allocator's deallocate function, instead of the size of the allocated memory. This happens to work for some allocator implementations that ignore the parameter (libstdc++'s malloc_allocator), but will lead to hard-to-diagnose segfaults on other architectures. I've observed a segfault on Linux + tcmalloc, and I guess this is the reason that allocators needed to be commented out to avoid segfaults on Windows. In any case, it's relying on undefined behaviour and a maintenance hazard.
I realize that both of these issues are not easily fixable. Since allocators are not supported yet anyway (#1061), I'd suggest to disable support for custom allocators entirely and depend on the default RCL allocator. If you concur, I'd be happy to help in implementation.