niklas/rclcpp
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix SEGV caused by order of destruction of Node sub-interfaces (#1469) (#1736)

Browse source 
* added tear-down of node sub-interfaces in reverse order of their creation (#1469)

Signed-off-by: Colin MacKenzie <colin@flyingeinstein.com>

* added name of service to log message for leak detection. Previously it gave no indication of what node is causing the memory leak (#1469)

Signed-off-by: Colin MacKenzie <colin@flyingeinstein.com>
(cherry picked from commit b9ffd72f42ae44352256f2eab585299bf85c6346)

Co-authored-by: Colin MacKenzie <guru-florida@users.noreply.github.com>
This commit is contained in:
mergify[bot] 2021-08-03 17:03:02 -03:00 • committed by GitHub
parent dc3832c4ec
commit 92fe787a74
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 28 additions and 6 deletions
Download patch file Download diff file Expand all files Collapse all files

8
rclcpp/include/rclcpp/service.hpp
View file

@ -180,7 +180,7 @@ public:
std::weak_ptr<rcl_node_t> weak_node_handle(node_handle_); std::weak_ptr<rcl_node_t> weak_node_handle(node_handle_);
// rcl does the static memory allocation here // rcl does the static memory allocation here
service_handle_ = std::shared_ptr<rcl_service_t>( service_handle_ = std::shared_ptr<rcl_service_t>(
new rcl_service_t, [weak_node_handle](rcl_service_t * service) new rcl_service_t, [weak_node_handle, service_name](rcl_service_t * service)
{ {
auto handle = weak_node_handle.lock(); auto handle = weak_node_handle.lock();
if (handle) { if (handle) {
@ -192,10 +192,10 @@ public:
rcl_reset_error(); rcl_reset_error();
} }
} else { } else {
RCLCPP_ERROR( RCLCPP_ERROR_STREAM(
rclcpp::get_logger("rclcpp"), rclcpp::get_logger("rclcpp"),
"Error in destruction of rcl service handle: " "Error in destruction of rcl service handle " << service_name <<
"the Node Handle was destructed too early. You will leak memory"); ": the Node Handle was destructed too early. You will leak memory");
} }
delete service; delete service;
}); });

13
rclcpp/src/rclcpp/node.cpp
View file

@ -184,7 +184,18 @@ Node::Node(
} }
Node::~Node() Node::~Node()
{} {
// release sub-interfaces in an order that allows them to consult with node_base during tear-down
node_waitables_.reset();
node_time_source_.reset();
node_parameters_.reset();
node_clock_.reset();
node_services_.reset();
node_topics_.reset();
node_timers_.reset();
node_logging_.reset();
node_graph_.reset();
}
const char * const char *
Node::get_name() const Node::get_name() const

13
rclcpp_lifecycle/src/lifecycle_node.cpp
View file

@ -126,7 +126,18 @@ LifecycleNode::LifecycleNode(
} }
LifecycleNode::~LifecycleNode() LifecycleNode::~LifecycleNode()
{} {
// release sub-interfaces in an order that allows them to consult with node_base during tear-down
node_waitables_.reset();
node_time_source_.reset();
node_parameters_.reset();
node_clock_.reset();
node_services_.reset();
node_topics_.reset();
node_timers_.reset();
node_logging_.reset();
node_graph_.reset();
}
const char * const char *
LifecycleNode::get_name() const LifecycleNode::get_name() const