Keep custom allocator in publisher and subscription options alive. (r…

…os2#1647)

* Keep custom allocator in publisher and subscription options alive.

Also, enforce an allocator bound to void is used to avoid surprises.

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>

* Avoid sizeof(void) in MyAllocator implementation.

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>

* Address peer review comment

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

* Use a lazely initialized private field when 'allocator' is not initialized

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Co-authored-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

rclcpp/include/rclcpp/publisher_options.hpp

@@ -17,6 +17,7 @@
 
 #include <memory>
 #include <string>
+#include <type_traits>
 #include <vector>
 
 #include "rcl/publisher.h"
@@ -64,6 +65,10 @@ struct PublisherOptionsBase
 template<typename Allocator>
 struct PublisherOptionsWithAllocator : public PublisherOptionsBase
 {
+ static_assert(
+ std::is_void_v<typename std::allocator_traits<Allocator>::value_type>,
+ "Publisher allocator value type must be void");
+
  /// Optional custom allocator.
  std::shared_ptr<Allocator> allocator = nullptr;
 
@@ -80,10 +85,7 @@ struct PublisherOptionsWithAllocator : public PublisherOptionsBase
  to_rcl_publisher_options(const rclcpp::QoS & qos) const
  {
  rcl_publisher_options_t result = rcl_publisher_get_default_options();
- using AllocatorTraits = std::allocator_traits<Allocator>;
- using MessageAllocatorT = typename AllocatorTraits::template rebind_alloc<MessageT>;
- auto message_alloc = std::make_shared<MessageAllocatorT>(*this->get_allocator().get());
- result.allocator = rclcpp::allocator::get_rcl_allocator<MessageT>(*message_alloc);
+ result.allocator = rclcpp::allocator::get_rcl_allocator<void>(*this->get_allocator());
  result.qos = qos.get_rmw_qos_profile();
  result.rmw_publisher_options.require_unique_network_flow_endpoints =
  this->require_unique_network_flow_endpoints;
@@ -102,10 +104,18 @@ struct PublisherOptionsWithAllocator : public PublisherOptionsBase
  get_allocator() const
  {
  if (!this->allocator) {
- return std::make_shared<Allocator>();
+ if (!allocator_storage_) {
+ allocator_storage_ = std::make_shared<Allocator>();
+ }
+ return allocator_storage_;
  }
  return this->allocator;
  }
+
+private:
+ // This is a temporal workaround, to make sure that get_allocator()
+ // always returns a copy of the same allocator.
+ mutable std::shared_ptr<Allocator> allocator_storage_;
 };
 
 using PublisherOptions = PublisherOptionsWithAllocator<std::allocator<void>>;

rclcpp/include/rclcpp/subscription_options.hpp

@@ -18,6 +18,7 @@
 #include <chrono>
 #include <memory>
 #include <string>
+#include <type_traits>
 #include <vector>
 
 #include "rclcpp/callback_group.hpp"
@@ -86,6 +87,10 @@ struct SubscriptionOptionsBase
 template<typename Allocator>
 struct SubscriptionOptionsWithAllocator : public SubscriptionOptionsBase
 {
+ static_assert(
+ std::is_void_v<typename std::allocator_traits<Allocator>::value_type>,
+ "Subscription allocator value type must be void");
+
  /// Optional custom allocator.
  std::shared_ptr<Allocator> allocator = nullptr;
 
@@ -107,10 +112,7 @@ struct SubscriptionOptionsWithAllocator : public SubscriptionOptionsBase
  to_rcl_subscription_options(const rclcpp::QoS & qos) const
  {
  rcl_subscription_options_t result = rcl_subscription_get_default_options();
- using AllocatorTraits = std::allocator_traits<Allocator>;
- using MessageAllocatorT = typename AllocatorTraits::template rebind_alloc<MessageT>;
- auto message_alloc = std::make_shared<MessageAllocatorT>(*this->get_allocator().get());
- result.allocator = allocator::get_rcl_allocator<MessageT>(*message_alloc);
+ result.allocator = allocator::get_rcl_allocator<void>(*this->get_allocator());
  result.qos = qos.get_rmw_qos_profile();
  result.rmw_subscription_options.ignore_local_publications = this->ignore_local_publications;
  result.rmw_subscription_options.require_unique_network_flow_endpoints =
@@ -124,15 +126,22 @@ struct SubscriptionOptionsWithAllocator : public SubscriptionOptionsBase
  return result;
  }
 
- /// Get the allocator, creating one if needed.
  std::shared_ptr<Allocator>
  get_allocator() const
  {
  if (!this->allocator) {
- return std::make_shared<Allocator>();
+ if (!allocator_storage_) {
+ allocator_storage_ = std::make_shared<Allocator>();
+ }
+ return allocator_storage_;
  }
  return this->allocator;
  }
+
+private:
+ // This is a temporal workaround, to make sure that get_allocator()
+ // always returns a copy of the same allocator.
+ mutable std::shared_ptr<Allocator> allocator_storage_;
 };
 
 using SubscriptionOptions = SubscriptionOptionsWithAllocator<std::allocator<void>>;

rclcpp/test/rclcpp/test_intra_process_manager_with_allocators.cpp

@@ -18,6 +18,7 @@
 #include <list>
 #include <memory>
 #include <string>
+#include <type_traits>
 #include <utility>
 
 #include "test_msgs/msg/empty.hpp"
@@ -57,7 +58,10 @@ struct MyAllocator
  return nullptr;
  }
  num_allocs++;
- return static_cast<T *>(std::malloc(size * sizeof(T)));
+ // Use sizeof(char) in place for sizeof(void)
+ constexpr size_t value_size = sizeof(
+ typename std::conditional<!std::is_void<T>::value, T, char>::type);
+ return static_cast<T *>(std::malloc(size * value_size));
  }
 
  void deallocate(T * ptr, size_t size)