Skip to content

Commit

Permalink
Move dynamic type support struct to rosidl and fix mem issues
Browse files Browse the repository at this point in the history
Signed-off-by: methylDragon <methylDragon@gmail.com>
  • Loading branch information
methylDragon committed Apr 5, 2023
1 parent 000f788 commit 0a21288
Show file tree
Hide file tree
Showing 7 changed files with 65 additions and 62 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ 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 = "");
Expand Down Expand Up @@ -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();
};


Expand Down
9 changes: 3 additions & 6 deletions rclcpp/src/rclcpp/dynamic_typesupport/dynamic_message.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
}

Expand Down Expand Up @@ -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);
});
}

Expand All @@ -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);
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
}

Expand All @@ -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);
});
}

Expand Down Expand Up @@ -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);
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
}

Expand Down Expand Up @@ -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);
});
}

Expand Down Expand Up @@ -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);
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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;
}

Expand All @@ -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;
}
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 0a21288

Please sign in to comment.