diff --git a/rclcpp/include/rclcpp/client.hpp b/rclcpp/include/rclcpp/client.hpp index e88fa8a949..04e0ce9a33 100644 --- a/rclcpp/include/rclcpp/client.hpp +++ b/rclcpp/include/rclcpp/client.hpp @@ -130,7 +130,7 @@ class ClientBase rclcpp::node_interfaces::NodeGraphInterface::SharedPtr node_graph); RCLCPP_PUBLIC - virtual ~ClientBase(); + virtual ~ClientBase() = default; /// Take the next response for this client as a type erased pointer. /** diff --git a/rclcpp/include/rclcpp/experimental/subscription_intra_process_base.hpp b/rclcpp/include/rclcpp/experimental/subscription_intra_process_base.hpp index 037d1aaa2a..6583e74ae7 100644 --- a/rclcpp/include/rclcpp/experimental/subscription_intra_process_base.hpp +++ b/rclcpp/include/rclcpp/experimental/subscription_intra_process_base.hpp @@ -52,7 +52,7 @@ class SubscriptionIntraProcessBase : public rclcpp::Waitable {} RCLCPP_PUBLIC - virtual ~SubscriptionIntraProcessBase(); + virtual ~SubscriptionIntraProcessBase() = default; RCLCPP_PUBLIC size_t diff --git a/rclcpp/include/rclcpp/service.hpp b/rclcpp/include/rclcpp/service.hpp index 3f500eaa09..65b457ffda 100644 --- a/rclcpp/include/rclcpp/service.hpp +++ b/rclcpp/include/rclcpp/service.hpp @@ -55,7 +55,7 @@ class ServiceBase explicit ServiceBase(std::shared_ptr node_handle); RCLCPP_PUBLIC - virtual ~ServiceBase(); + virtual ~ServiceBase() = default; /// Return the name of the service. /** \return The name of the service. */ diff --git a/rclcpp/src/rclcpp/client.cpp b/rclcpp/src/rclcpp/client.cpp index 17cc68e153..4adc6d4a96 100644 --- a/rclcpp/src/rclcpp/client.cpp +++ b/rclcpp/src/rclcpp/client.cpp @@ -65,13 +65,6 @@ ClientBase::ClientBase( }); } -ClientBase::~ClientBase() -{ - clear_on_new_response_callback(); - // Make sure the client handle is destructed as early as possible and before the node handle - client_handle_.reset(); -} - bool ClientBase::take_type_erased_response(void * response_out, rmw_request_id_t & request_header_out) { diff --git a/rclcpp/src/rclcpp/publisher_base.cpp b/rclcpp/src/rclcpp/publisher_base.cpp index 723d56e0d5..9517af6d3c 100644 --- a/rclcpp/src/rclcpp/publisher_base.cpp +++ b/rclcpp/src/rclcpp/publisher_base.cpp @@ -102,11 +102,6 @@ PublisherBase::PublisherBase( PublisherBase::~PublisherBase() { - for (const auto & pair : event_handlers_) { - rcl_publisher_event_type_t event_type = pair.first; - clear_on_new_qos_event_callback(event_type); - } - // must fini the events before fini-ing the publisher event_handlers_.clear(); diff --git a/rclcpp/src/rclcpp/qos_event.cpp b/rclcpp/src/rclcpp/qos_event.cpp index 8bfd5b3304..91df5bb346 100644 --- a/rclcpp/src/rclcpp/qos_event.cpp +++ b/rclcpp/src/rclcpp/qos_event.cpp @@ -35,6 +35,11 @@ UnsupportedEventTypeException::UnsupportedEventTypeException( QOSEventHandlerBase::~QOSEventHandlerBase() { + // Since the rmw event listener holds a reference to + // this callback, we need to clear it on destruction of this class. + // This clearing is not needed for other rclcpp entities like pub/subs, since + // they do own the underlying rmw entities, which are destroyed + // on their rclcpp destructors, thus no risk of dangling pointers. if (on_new_event_callback_) { clear_on_ready_callback(); } diff --git a/rclcpp/src/rclcpp/service.cpp b/rclcpp/src/rclcpp/service.cpp index 7bc580ce68..ea1a83a2a6 100644 --- a/rclcpp/src/rclcpp/service.cpp +++ b/rclcpp/src/rclcpp/service.cpp @@ -32,10 +32,6 @@ ServiceBase::ServiceBase(std::shared_ptr node_handle) node_logger_(rclcpp::get_node_logger(node_handle_.get())) {} -ServiceBase::~ServiceBase() -{ - clear_on_new_request_callback(); -} bool ServiceBase::take_type_erased_request(void * request_out, rmw_request_id_t & request_id_out) diff --git a/rclcpp/src/rclcpp/subscription_base.cpp b/rclcpp/src/rclcpp/subscription_base.cpp index 300f465a41..871321fbe8 100644 --- a/rclcpp/src/rclcpp/subscription_base.cpp +++ b/rclcpp/src/rclcpp/subscription_base.cpp @@ -87,13 +87,6 @@ SubscriptionBase::SubscriptionBase( SubscriptionBase::~SubscriptionBase() { - clear_on_new_message_callback(); - - for (const auto & pair : event_handlers_) { - rcl_subscription_event_type_t event_type = pair.first; - clear_on_new_qos_event_callback(event_type); - } - if (!use_intra_process_) { return; } diff --git a/rclcpp/src/rclcpp/subscription_intra_process_base.cpp b/rclcpp/src/rclcpp/subscription_intra_process_base.cpp index 21ccb433ff..b13123b65b 100644 --- a/rclcpp/src/rclcpp/subscription_intra_process_base.cpp +++ b/rclcpp/src/rclcpp/subscription_intra_process_base.cpp @@ -17,11 +17,6 @@ using rclcpp::experimental::SubscriptionIntraProcessBase; -SubscriptionIntraProcessBase::~SubscriptionIntraProcessBase() -{ - clear_on_ready_callback(); -} - void SubscriptionIntraProcessBase::add_to_wait_set(rcl_wait_set_t * wait_set) { diff --git a/rclcpp_action/src/client.cpp b/rclcpp_action/src/client.cpp index f9144ecf49..2266b7cee6 100644 --- a/rclcpp_action/src/client.cpp +++ b/rclcpp_action/src/client.cpp @@ -136,7 +136,6 @@ ClientBase::ClientBase( ClientBase::~ClientBase() { - clear_on_ready_callback(); } bool diff --git a/rclcpp_action/src/server.cpp b/rclcpp_action/src/server.cpp index b0afb9aa50..42c4074495 100644 --- a/rclcpp_action/src/server.cpp +++ b/rclcpp_action/src/server.cpp @@ -132,7 +132,6 @@ ServerBase::ServerBase( ServerBase::~ServerBase() { - clear_on_ready_callback(); } size_t