diff --git a/rclcpp/include/rclcpp/allocator/allocator_common.hpp b/rclcpp/include/rclcpp/allocator/allocator_common.hpp index 12b2f383b6..f1d411d25f 100644 --- a/rclcpp/include/rclcpp/allocator/allocator_common.hpp +++ b/rclcpp/include/rclcpp/allocator/allocator_common.hpp @@ -30,14 +30,19 @@ namespace allocator template using AllocRebind = typename std::allocator_traits::template rebind_traits; -template -void * retyped_allocate(size_t size, void * untyped_allocator) +/// Return the equivalent rcl_allocator_t for the C++ standard allocator. +/** + * This assumes that the user intent behind both allocators is the + * same: Using system malloc for allocation. + * + * If you're using a custom allocator in ROS, you'll need to provide + * your own overload for this function. + */ +template +rcl_allocator_t get_rcl_allocator(std::allocator allocator) { - auto typed_allocator = static_cast(untyped_allocator); - if (!typed_allocator) { - throw std::runtime_error("Received incorrect allocator type"); - } - return std::allocator_traits::allocate(*typed_allocator, size); + (void)allocator; // Remove warning + return rcl_get_default_allocator(); } template @@ -56,61 +61,6 @@ void * retyped_zero_allocate(size_t number_of_elem, size_t size_of_elem, void * return allocated_memory; } -template -void retyped_deallocate(void * untyped_pointer, void * untyped_allocator) -{ - auto typed_allocator = static_cast(untyped_allocator); - if (!typed_allocator) { - throw std::runtime_error("Received incorrect allocator type"); - } - auto typed_ptr = static_cast(untyped_pointer); - std::allocator_traits::deallocate(*typed_allocator, typed_ptr, 1); -} - -template -void * retyped_reallocate(void * untyped_pointer, size_t size, void * untyped_allocator) -{ - auto typed_allocator = static_cast(untyped_allocator); - if (!typed_allocator) { - throw std::runtime_error("Received incorrect allocator type"); - } - auto typed_ptr = static_cast(untyped_pointer); - std::allocator_traits::deallocate(*typed_allocator, typed_ptr, 1); - return std::allocator_traits::allocate(*typed_allocator, size); -} - - -// Convert a std::allocator_traits-formatted Allocator into an rcl allocator -template< - typename T, - typename Alloc, - typename std::enable_if>::value>::type * = nullptr> -rcl_allocator_t get_rcl_allocator(Alloc & allocator) -{ - rcl_allocator_t rcl_allocator = rcl_get_default_allocator(); -#ifndef _WIN32 - rcl_allocator.allocate = &retyped_allocate; - rcl_allocator.zero_allocate = &retyped_zero_allocate; - rcl_allocator.deallocate = &retyped_deallocate; - rcl_allocator.reallocate = &retyped_reallocate; - rcl_allocator.state = &allocator; -#else - (void)allocator; // Remove warning -#endif - return rcl_allocator; -} - -// TODO(jacquelinekay) Workaround for an incomplete implementation of std::allocator -template< - typename T, - typename Alloc, - typename std::enable_if>::value>::type * = nullptr> -rcl_allocator_t get_rcl_allocator(Alloc & allocator) -{ - (void)allocator; - return rcl_get_default_allocator(); -} - } // namespace allocator } // namespace rclcpp diff --git a/rclcpp/include/rclcpp/message_memory_strategy.hpp b/rclcpp/include/rclcpp/message_memory_strategy.hpp index f548d953c2..d664239b25 100644 --- a/rclcpp/include/rclcpp/message_memory_strategy.hpp +++ b/rclcpp/include/rclcpp/message_memory_strategy.hpp @@ -18,6 +18,7 @@ #include #include +#include "rcl/allocator.h" #include "rcl/types.h" #include "rclcpp/allocator/allocator_common.hpp" @@ -61,7 +62,7 @@ class MessageMemoryStrategy message_allocator_ = std::make_shared(); serialized_message_allocator_ = std::make_shared(); buffer_allocator_ = std::make_shared(); - rcutils_allocator_ = allocator::get_rcl_allocator(*buffer_allocator_.get()); + rcutils_allocator_ = rcl_get_default_allocator(); } explicit MessageMemoryStrategy(std::shared_ptr allocator) @@ -69,7 +70,7 @@ class MessageMemoryStrategy message_allocator_ = std::make_shared(*allocator.get()); serialized_message_allocator_ = std::make_shared(*allocator.get()); buffer_allocator_ = std::make_shared(*allocator.get()); - rcutils_allocator_ = allocator::get_rcl_allocator(*buffer_allocator_.get()); + rcutils_allocator_ = rcl_get_default_allocator(); } virtual ~MessageMemoryStrategy() = default; diff --git a/rclcpp/include/rclcpp/publisher_options.hpp b/rclcpp/include/rclcpp/publisher_options.hpp index 3c88ebccd1..04d943265d 100644 --- a/rclcpp/include/rclcpp/publisher_options.hpp +++ b/rclcpp/include/rclcpp/publisher_options.hpp @@ -20,6 +20,7 @@ #include #include +#include "rcl/allocator.h" #include "rcl/publisher.h" #include "rclcpp/allocator/allocator_common.hpp" @@ -77,7 +78,8 @@ struct PublisherOptionsWithAllocator : public PublisherOptionsBase /// Constructor using base class as input. explicit PublisherOptionsWithAllocator(const PublisherOptionsBase & publisher_options_base) : PublisherOptionsBase(publisher_options_base) - {} + { + } /// Convert this class, and a rclcpp::QoS, into an rcl_publisher_options_t. template @@ -85,7 +87,7 @@ struct PublisherOptionsWithAllocator : public PublisherOptionsBase to_rcl_publisher_options(const rclcpp::QoS & qos) const { rcl_publisher_options_t result = rcl_publisher_get_default_options(); - result.allocator = this->get_rcl_allocator(); + result.allocator = rcl_get_default_allocator(); result.qos = qos.get_rmw_qos_profile(); result.rmw_publisher_options.require_unique_network_flow_endpoints = this->require_unique_network_flow_endpoints; @@ -98,7 +100,6 @@ struct PublisherOptionsWithAllocator : public PublisherOptionsBase return result; } - /// Get the allocator, creating one if needed. std::shared_ptr get_allocator() const @@ -123,7 +124,7 @@ struct PublisherOptionsWithAllocator : public PublisherOptionsBase plain_allocator_storage_ = std::make_shared(*this->get_allocator()); } - return rclcpp::allocator::get_rcl_allocator(*plain_allocator_storage_); + return rcl_get_default_allocator(); } // This is a temporal workaround, to make sure that get_allocator() diff --git a/rclcpp/include/rclcpp/strategies/allocator_memory_strategy.hpp b/rclcpp/include/rclcpp/strategies/allocator_memory_strategy.hpp index 88698179d4..db8a59f5c7 100644 --- a/rclcpp/include/rclcpp/strategies/allocator_memory_strategy.hpp +++ b/rclcpp/include/rclcpp/strategies/allocator_memory_strategy.hpp @@ -424,7 +424,7 @@ class AllocatorMemoryStrategy : public memory_strategy::MemoryStrategy rcl_allocator_t get_allocator() override { - return rclcpp::allocator::get_rcl_allocator(*allocator_.get()); + return rcl_get_default_allocator(); } size_t number_of_ready_subscriptions() const override diff --git a/rclcpp/include/rclcpp/subscription_options.hpp b/rclcpp/include/rclcpp/subscription_options.hpp index 2b819da399..5f9dbf5ec7 100644 --- a/rclcpp/include/rclcpp/subscription_options.hpp +++ b/rclcpp/include/rclcpp/subscription_options.hpp @@ -21,6 +21,8 @@ #include #include +#include "rcl/allocator.h" + #include "rclcpp/callback_group.hpp" #include "rclcpp/detail/rmw_implementation_specific_subscription_payload.hpp" #include "rclcpp/intra_process_buffer_type.hpp" @@ -103,7 +105,8 @@ struct SubscriptionOptionsWithAllocator : public SubscriptionOptionsBase explicit SubscriptionOptionsWithAllocator( const SubscriptionOptionsBase & subscription_options_base) : SubscriptionOptionsBase(subscription_options_base) - {} + { + } /// Convert this class, with a rclcpp::QoS, into an rcl_subscription_options_t. /** @@ -115,7 +118,7 @@ struct SubscriptionOptionsWithAllocator : public SubscriptionOptionsBase to_rcl_subscription_options(const rclcpp::QoS & qos) const { rcl_subscription_options_t result = rcl_subscription_get_default_options(); - result.allocator = this->get_rcl_allocator(); + result.allocator = rcl_get_default_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 = @@ -167,7 +170,7 @@ struct SubscriptionOptionsWithAllocator : public SubscriptionOptionsBase plain_allocator_storage_ = std::make_shared(*this->get_allocator()); } - return rclcpp::allocator::get_rcl_allocator(*plain_allocator_storage_); + return rcl_get_default_allocator(); } // This is a temporal workaround, to make sure that get_allocator() diff --git a/rclcpp/test/rclcpp/allocator/test_allocator_common.cpp b/rclcpp/test/rclcpp/allocator/test_allocator_common.cpp index 4619b7665d..5e1d8bc90c 100644 --- a/rclcpp/test/rclcpp/allocator/test_allocator_common.cpp +++ b/rclcpp/test/rclcpp/allocator/test_allocator_common.cpp @@ -18,54 +18,17 @@ #include "rclcpp/allocator/allocator_common.hpp" -TEST(TestAllocatorCommon, retyped_allocate) { - std::allocator allocator; - void * untyped_allocator = &allocator; - void * allocated_mem = - rclcpp::allocator::retyped_allocate>(1u, untyped_allocator); - // The more natural check here is ASSERT_NE(nullptr, ptr), but clang static - // analysis throws a false-positive memory leak warning. Use ASSERT_TRUE instead. - ASSERT_TRUE(nullptr != allocated_mem); - - auto code = [&untyped_allocator, allocated_mem]() { - rclcpp::allocator::retyped_deallocate>( - allocated_mem, untyped_allocator); - }; - EXPECT_NO_THROW(code()); - - allocated_mem = allocator.allocate(1); - // The more natural check here is ASSERT_NE(nullptr, ptr), but clang static - // analysis throws a false-positive memory leak warning. Use ASSERT_TRUE instead. - ASSERT_TRUE(nullptr != allocated_mem); - void * reallocated_mem = - rclcpp::allocator::retyped_reallocate>( - allocated_mem, 2u, untyped_allocator); - // The more natural check here is ASSERT_NE(nullptr, ptr), but clang static - // analysis throws a false-positive memory leak warning. Use ASSERT_TRUE instead. - ASSERT_TRUE(nullptr != reallocated_mem); - - auto code2 = [&untyped_allocator, reallocated_mem]() { - rclcpp::allocator::retyped_deallocate>( - reallocated_mem, untyped_allocator); - }; - EXPECT_NO_THROW(code2()); -} - -TEST(TestAllocatorCommon, retyped_zero_allocate_basic) { +TEST(TestAllocatorCommon, retyped_zero_allocate_basic) +{ std::allocator allocator; void * untyped_allocator = &allocator; void * allocated_mem = rclcpp::allocator::retyped_zero_allocate>(20u, 1u, untyped_allocator); ASSERT_TRUE(nullptr != allocated_mem); - - auto code = [&untyped_allocator, allocated_mem]() { - rclcpp::allocator::retyped_deallocate>( - allocated_mem, untyped_allocator); - }; - EXPECT_NO_THROW(code()); } -TEST(TestAllocatorCommon, retyped_zero_allocate) { +TEST(TestAllocatorCommon, retyped_zero_allocate) +{ std::allocator allocator; void * untyped_allocator = &allocator; void * allocated_mem = @@ -73,34 +36,12 @@ TEST(TestAllocatorCommon, retyped_zero_allocate) { // The more natural check here is ASSERT_NE(nullptr, ptr), but clang static // analysis throws a false-positive memory leak warning. Use ASSERT_TRUE instead. ASSERT_TRUE(nullptr != allocated_mem); - - auto code = [&untyped_allocator, allocated_mem]() { - rclcpp::allocator::retyped_deallocate>( - allocated_mem, untyped_allocator); - }; - EXPECT_NO_THROW(code()); - - allocated_mem = allocator.allocate(1); - // The more natural check here is ASSERT_NE(nullptr, ptr), but clang static - // analysis throws a false-positive memory leak warning. Use ASSERT_TRUE instead. - ASSERT_TRUE(nullptr != allocated_mem); - void * reallocated_mem = - rclcpp::allocator::retyped_reallocate>( - allocated_mem, 2u, untyped_allocator); - // The more natural check here is ASSERT_NE(nullptr, ptr), but clang static - // analysis throws a false-positive memory leak warning. Use ASSERT_TRUE instead. - ASSERT_TRUE(nullptr != reallocated_mem); - - auto code2 = [&untyped_allocator, reallocated_mem]() { - rclcpp::allocator::retyped_deallocate>( - reallocated_mem, untyped_allocator); - }; - EXPECT_NO_THROW(code2()); } -TEST(TestAllocatorCommon, get_rcl_allocator) { +TEST(TestAllocatorCommon, get_rcl_allocator) +{ std::allocator allocator; - auto rcl_allocator = rclcpp::allocator::get_rcl_allocator(allocator); + auto rcl_allocator = rclcpp::allocator::get_rcl_allocator(allocator); EXPECT_NE(nullptr, rcl_allocator.allocate); EXPECT_NE(nullptr, rcl_allocator.deallocate); EXPECT_NE(nullptr, rcl_allocator.reallocate); @@ -108,10 +49,10 @@ TEST(TestAllocatorCommon, get_rcl_allocator) { // Not testing state as that may or may not be null depending on platform } -TEST(TestAllocatorCommon, get_void_rcl_allocator) { +TEST(TestAllocatorCommon, get_void_rcl_allocator) +{ std::allocator allocator; - auto rcl_allocator = - rclcpp::allocator::get_rcl_allocator>(allocator); + auto rcl_allocator = rclcpp::allocator::get_rcl_allocator(allocator); EXPECT_NE(nullptr, rcl_allocator.allocate); EXPECT_NE(nullptr, rcl_allocator.deallocate); EXPECT_NE(nullptr, rcl_allocator.reallocate);