Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to use custom allocator with LoanedMessage #2671

Open
thomasmoore-torc opened this issue Nov 12, 2024 · 1 comment · May be fixed by #2672
Open

Unable to use custom allocator with LoanedMessage #2671

thomasmoore-torc opened this issue Nov 12, 2024 · 1 comment · May be fixed by #2672

Comments

@thomasmoore-torc
Copy link

Bug report

Required Info:

  • Operating System:
    • Linux
  • Installation type:
    • From source
  • Version or commit hash:
    • iron, jazzy, rolling
  • DDS implementation:
    • Fast-RTPS
  • Client library (if applicable):
    • rclcpp

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 fujitatomoya linked a pull request Nov 13, 2024 that will close this issue
@fujitatomoya
Copy link
Collaborator

@thomasmoore-torc as always, thanks for sending us a patch and good description 👍 i created #2672, if you can review, that would be really appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants