Move dynamic type support struct to rosidl and fix mem issues

Signed-off-by: methylDragon <[email protected]>

Author: methylDragon

rclcpp/include/rclcpp/dynamic_typesupport/dynamic_message_type_support.hpp

@@ -43,7 +43,7 @@ namespace dynamic_typesupport
  *       support struct, instead of rcl_dynamic_message_type_support_handle_init,
  *       because this class will manage the lifetimes for you.
  *
- *       Do NOT call rcl_dynamic_message_type_support_handle_fini!!
+ *       Do NOT call rcl_dynamic_message_type_support_handle_destroy!!
  *
  * This class:
  * - Manages the lifetime of the raw pointer.
@@ -178,10 +178,6 @@ class DynamicMessageTypeSupport : public std::enable_shared_from_this<DynamicMes
   RCLCPP_PUBLIC
   DynamicMessageTypeSupport();
 
-  RCLCPP_PUBLIC
-  void
-  manage_rosidl_message_type_support_(rosidl_message_type_support_t * rosidl_message_type_support);
-
   RCLCPP_PUBLIC
   void
   manage_description_(rosidl_runtime_c__type_description__TypeDescription * description);

rclcpp/include/rclcpp/dynamic_typesupport/dynamic_serialization_support.hpp

@@ -46,9 +46,12 @@ class DynamicSerializationSupport : public std::enable_shared_from_this<DynamicS
   RCLCPP_SMART_PTR_ALIASES_ONLY(DynamicSerializationSupport)
 
   // CONSTRUCTION ==================================================================================
+  RCLCPP_PUBLIC
+  DynamicSerializationSupport();
+
   /// Get the rmw middleware implementation specific serialization support (configured by name)
   RCLCPP_PUBLIC
-  explicit DynamicSerializationSupport(const std::string & serialization_library_name = "");
+  explicit DynamicSerializationSupport(const std::string & serialization_library_name);
 
   /// Assume ownership of raw pointer
   RCLCPP_PUBLIC
@@ -97,10 +100,6 @@ class DynamicSerializationSupport : public std::enable_shared_from_this<DynamicS
   RCLCPP_DISABLE_COPY(DynamicSerializationSupport)
 
   std::shared_ptr<rosidl_dynamic_typesupport_serialization_support_t> rosidl_serialization_support_;
-
-private:
-  RCLCPP_PUBLIC
-  DynamicSerializationSupport();
 };
 
 

rclcpp/src/rclcpp/dynamic_typesupport/dynamic_message.cpp

@@ -68,8 +68,7 @@ DynamicMessage::DynamicMessage(const DynamicMessageTypeBuilder::SharedPtr dynami
     rosidl_dynamic_data,
     // Custom deleter
     [](rosidl_dynamic_typesupport_dynamic_data_t * rosidl_dynamic_data)->void {
-      rosidl_dynamic_typesupport_dynamic_data_fini(rosidl_dynamic_data);
-      delete rosidl_dynamic_data;
+      rosidl_dynamic_typesupport_dynamic_data_destroy(rosidl_dynamic_data);
     });
 }
 
@@ -103,8 +102,7 @@ DynamicMessage::DynamicMessage(const DynamicMessageType::SharedPtr dynamic_type)
     rosidl_dynamic_data,
     // Custom deleter
     [](rosidl_dynamic_typesupport_dynamic_data_t * rosidl_dynamic_data)->void {
-      rosidl_dynamic_typesupport_dynamic_data_fini(rosidl_dynamic_data);
-      delete rosidl_dynamic_data;
+      rosidl_dynamic_typesupport_dynamic_data_destroy(rosidl_dynamic_data);
     });
 }
 
@@ -131,8 +129,7 @@ DynamicMessage::DynamicMessage(
     rosidl_dynamic_data,
     // Custom deleter
     [](rosidl_dynamic_typesupport_dynamic_data_t * rosidl_dynamic_data)->void {
-      rosidl_dynamic_typesupport_dynamic_data_fini(rosidl_dynamic_data);
-      delete rosidl_dynamic_data;
+      rosidl_dynamic_typesupport_dynamic_data_destroy(rosidl_dynamic_data);
     });
 }
 

rclcpp/src/rclcpp/dynamic_typesupport/dynamic_message_type.cpp

@@ -60,8 +60,7 @@ DynamicMessageType::DynamicMessageType(DynamicMessageTypeBuilder::SharedPtr dyna
     rosidl_dynamic_type,
     // Custom deleter
     [](rosidl_dynamic_typesupport_dynamic_type_t * rosidl_dynamic_type)->void {
-      rosidl_dynamic_typesupport_dynamic_type_fini(rosidl_dynamic_type);
-      delete rosidl_dynamic_type;
+      rosidl_dynamic_typesupport_dynamic_type_destroy(rosidl_dynamic_type);
     });
 }
 
@@ -85,8 +84,7 @@ DynamicMessageType::DynamicMessageType(
     rosidl_dynamic_type,
     // Custom deleter
     [](rosidl_dynamic_typesupport_dynamic_type_t * rosidl_dynamic_type)->void {
-      rosidl_dynamic_typesupport_dynamic_type_fini(rosidl_dynamic_type);
-      delete rosidl_dynamic_type;
+      rosidl_dynamic_typesupport_dynamic_type_destroy(rosidl_dynamic_type);
     });
 }
 
@@ -171,8 +169,7 @@ DynamicMessageType::init_from_description(
     rosidl_dynamic_type,
     // Custom deleter
     [](rosidl_dynamic_typesupport_dynamic_type_t * rosidl_dynamic_type)->void {
-      rosidl_dynamic_typesupport_dynamic_type_fini(rosidl_dynamic_type);
-      delete rosidl_dynamic_type;
+      rosidl_dynamic_typesupport_dynamic_type_destroy(rosidl_dynamic_type);
     });
 }
 

rclcpp/src/rclcpp/dynamic_typesupport/dynamic_message_type_builder.cpp

@@ -72,8 +72,7 @@ DynamicMessageTypeBuilder::DynamicMessageTypeBuilder(
     rosidl_dynamic_type_builder,
     // Custom deleter
     [](rosidl_dynamic_typesupport_dynamic_type_builder_t * rosidl_dynamic_type_builder)->void {
-      rosidl_dynamic_typesupport_dynamic_type_builder_fini(rosidl_dynamic_type_builder);
-      delete rosidl_dynamic_type_builder;
+      rosidl_dynamic_typesupport_dynamic_type_builder_destroy(rosidl_dynamic_type_builder);
     });
 }
 
@@ -165,8 +164,7 @@ DynamicMessageTypeBuilder::init_from_description(
     rosidl_dynamic_type_builder,
     // Custom deleter
     [](rosidl_dynamic_typesupport_dynamic_type_builder_t * rosidl_dynamic_type_builder)->void {
-      rosidl_dynamic_typesupport_dynamic_type_builder_fini(rosidl_dynamic_type_builder);
-      delete rosidl_dynamic_type_builder;
+      rosidl_dynamic_typesupport_dynamic_type_builder_destroy(rosidl_dynamic_type_builder);
     });
 }
 
@@ -201,8 +199,7 @@ DynamicMessageTypeBuilder::init_from_serialization_support_(
     rosidl_dynamic_type_builder,
     // Custom deleter
     [](rosidl_dynamic_typesupport_dynamic_type_builder_t * rosidl_dynamic_type_builder)->void {
-      rosidl_dynamic_typesupport_dynamic_type_builder_fini(rosidl_dynamic_type_builder);
-      delete rosidl_dynamic_type_builder;
+      rosidl_dynamic_typesupport_dynamic_type_builder_destroy(rosidl_dynamic_type_builder);
     });
 }
 

rclcpp/src/rclcpp/dynamic_typesupport/dynamic_message_type_support.cpp

@@ -21,6 +21,7 @@
 #include <memory>
 #include <string>
 
+#include "rcl/allocator.h"
 #include "rcl/dynamic_message_type_support.h"
 #include "rcl/type_hash.h"
 #include "rcl/types.h"
@@ -76,9 +77,24 @@ DynamicMessageTypeSupport::DynamicMessageTypeSupport(
 
   // NOTE(methylDragon): Not technically const correct, but since it's a const void *,
   //                     we do it anyway...
-  auto ts_impl = static_cast<const rmw_dynamic_message_type_support_impl_t *>(ts->data);
+  auto ts_impl = static_cast<const rosidl_dynamic_message_type_support_impl_t *>(ts->data);
+
+  // NOTE(methylDragon): We don't destroy the rosidl_message_type_support->data since its members
+  //                     are managed by the passed in SharedPtr wrapper classes. We just delete it.
+  rosidl_message_type_support_.reset(
+    ts,
+    [](rosidl_message_type_support_t * ts) -> void {
+      auto ts_impl = static_cast<const rosidl_dynamic_message_type_support_impl_t *>(ts->data);
+      auto allocator = rcl_get_default_allocator();
+
+      // These are all C allocated
+      allocator.deallocate(ts_impl->type_hash, &allocator.state);
+      allocator.deallocate(
+        const_cast<rosidl_dynamic_message_type_support_impl_t *>(ts_impl), &allocator.state);
+      allocator.deallocate(ts, &allocator.state);
+    }
+  );
 
-  manage_rosidl_message_type_support_(ts);
   manage_description_(ts_impl->type_description);
   serialization_support_ = DynamicSerializationSupport::make_shared(ts_impl->serialization_support);
 
@@ -136,9 +152,21 @@ DynamicMessageTypeSupport::DynamicMessageTypeSupport(
     throw std::runtime_error("rosidl message type support is of the wrong type");
   }
 
-  auto ts_impl = static_cast<const rmw_dynamic_message_type_support_impl_t *>(ts->data);
+  auto ts_impl = static_cast<const rosidl_dynamic_message_type_support_impl_t *>(ts->data);
+
+  // NOTE(methylDragon): We don't finalize the rosidl_message_type_support->data since its members
+  //                     are managed by the passed in SharedPtr wrapper classes. We just delete it.
+  rosidl_message_type_support_.reset(
+    ts,
+    [](rosidl_message_type_support_t * ts) -> void {
+      auto ts_impl = static_cast<const rosidl_dynamic_message_type_support_impl_t *>(ts->data);
 
-  manage_rosidl_message_type_support_(ts);
+      // These are allocated with new
+      delete ts_impl->type_hash;  // Only because we should've allocated it here
+      delete ts_impl;
+      delete ts;
+    }
+  );
   manage_description_(ts_impl->type_description);
 
   dynamic_message_type_ = DynamicMessageType::make_shared(
@@ -237,24 +265,6 @@ DynamicMessageTypeSupport::DynamicMessageTypeSupport(
 DynamicMessageTypeSupport::~DynamicMessageTypeSupport() {}
 
 
-void
-DynamicMessageTypeSupport::manage_rosidl_message_type_support_(
-  rosidl_message_type_support_t * rosidl_message_type_support)
-{
-  // NOTE(methylDragon): We don't finalize the rosidl_message_type_support->data since its members
-  //                     are managed by the passed in SharedPtr wrapper classes. We just delete it.
-  rosidl_message_type_support_.reset(
-    rosidl_message_type_support,
-    [](rosidl_message_type_support_t * ts) -> void {
-      auto ts_impl = static_cast<const rmw_dynamic_message_type_support_impl_t *>(ts->data);
-      delete ts_impl->type_hash;  // Only because we should've allocated it here
-      delete ts_impl;
-      delete ts;
-    }
-  );
-}
-
-
 void
 DynamicMessageTypeSupport::manage_description_(
   rosidl_runtime_c__type_description__TypeDescription * description)
@@ -299,16 +309,18 @@ DynamicMessageTypeSupport::init_rosidl_message_type_support_(
     throw std::runtime_error("failed to get type hash");
   }
 
-  rmw_dynamic_message_type_support_impl_t * ts_impl = new rmw_dynamic_message_type_support_impl_t{
-    type_hash.get(),                                            // type_hash
-    description,                                                // type_description
-    nullptr,  // NOTE(methylDragon): Not supported for now      // type_description_sources
-    serialization_support->get_rosidl_serialization_support(),  // serialization_support
-    dynamic_message_type->get_rosidl_dynamic_type(),            // dynamic_message_type
-    dynamic_message->get_rosidl_dynamic_data()                  // dynamic_message
+  rosidl_dynamic_message_type_support_impl_t * ts_impl =
+    new rosidl_dynamic_message_type_support_impl_t {
+    type_hash.get(),                                              // type_hash
+    description,                                                  // type_description
+    nullptr,    // NOTE(methylDragon): Not supported for now      // type_description_sources
+    serialization_support->get_rosidl_serialization_support(),    // serialization_support
+    dynamic_message_type->get_rosidl_dynamic_type(),              // dynamic_message_type
+    dynamic_message->get_rosidl_dynamic_data()                    // dynamic_message
   };
   if (!ts_impl) {
-    throw std::runtime_error("Could not allocate rmw_dynamic_message_type_support_impl_t struct");
+    throw std::runtime_error(
+            "Could not allocate rosidl_dynamic_message_type_support_impl_t struct");
     return;
   }
 
@@ -320,15 +332,17 @@ DynamicMessageTypeSupport::init_rosidl_message_type_support_(
     ts_impl,                                              // data
     get_message_typesupport_handle_function,              // func
     // get_type_hash_func
-    rmw_dynamic_message_type_support_get_type_hash_function,
+    rosidl_get_dynamic_message_type_support_type_hash_function,
     // get_type_description_func
-    rmw_dynamic_message_type_support_get_type_description_function,
+    rosidl_get_dynamic_message_type_support_type_description_function,
     // get_type_description_sources_func
-    rmw_dynamic_message_type_support_get_type_description_sources_function
+    rosidl_get_dynamic_message_type_support_type_description_sources_function
   },
     [](rosidl_message_type_support_t * ts) -> void {
-      auto ts_impl = static_cast<const rmw_dynamic_message_type_support_impl_t *>(ts->data);
-      delete ts_impl->type_hash;  // Only because we should've allocated it here
+      auto ts_impl = static_cast<const rosidl_dynamic_message_type_support_impl_t *>(ts->data);
+      auto allocator = rcl_get_default_allocator();
+      // Only because we should've allocated it here (also it's C allocated)
+      allocator.deallocate(ts_impl->type_hash, &allocator.state);
       delete ts_impl;
     }
   );

rclcpp/src/rclcpp/dynamic_typesupport/dynamic_serialization_support.cpp

@@ -28,6 +28,9 @@
 using rclcpp::dynamic_typesupport::DynamicSerializationSupport;
 
 // CONSTRUCTION ====================================================================================
+DynamicSerializationSupport::DynamicSerializationSupport()
+: DynamicSerializationSupport::DynamicSerializationSupport("") {};
+
 DynamicSerializationSupport::DynamicSerializationSupport(
   const std::string & serialization_library_name)
 : rosidl_serialization_support_(nullptr)