ros2 / rclcpp

rclcpp (ROS Client Library for C++)
Apache License 2.0
559 stars 422 forks source link

Unable to use custom allocator with LoanedMessage #2671

Open thomasmoore-torc opened 2 days ago

thomasmoore-torc commented 2 days ago

Bug report

Required Info:

Steps to reproduce issue

The LoanedMessage class is defined with an AllocatorT template parameter which is used as the type for the message_allocator_ member variable:

template<typename MessageT, typename AllocatorT = std::allocator<void>>
class LoanedMessage
{
  using MessageAllocatorTraits = rclcpp::allocator::AllocRebind<MessageT, AllocatorT>;
  using MessageAllocator = typename MessageAllocatorTraits::allocator_type;

  MessageAllocator message_allocator_;
};

However, the constructor accepts an allocator parameter which is of type std::allocator<MessageT>:

  LoanedMessage(
    const rclcpp::PublisherBase & pub,
    MessageAllocator allocator)
  : pub_(pub),
    message_(nullptr),
    message_allocator_(std::move(allocator))
  {
  }

This prevents creating instances of LoanedMessage when using a custom allocator which is not based on std::allocator. Our recommendation to resolve this issue is to modify the constructor to use MessageAllocator as the type for the allocator parameter:

   LoanedMessage(
     const rclcpp::PublisherBase & pub,
-    std::allocator<MessageT> allocator)
+    MessageAllocator allocator)
   : pub_(pub),
     message_(nullptr),
     message_allocator_(std::move(allocator))
   {
   }
fujitatomoya commented 2 days ago

@thomasmoore-torc as always, thanks for sending us a patch and good description 👍 i created https://github.com/ros2/rclcpp/pull/2672, if you can review, that would be really appreciated.