Skip to content

Commit b9ffd72

Browse files
authored
Fix SEGV caused by order of destruction of Node sub-interfaces (ros2#1469)
* added tear-down of node sub-interfaces in reverse order of their creation (ros2#1469) Signed-off-by: Colin MacKenzie <[email protected]> * added name of service to log message for leak detection. Previously it gave no indication of what node is causing the memory leak (ros2#1469) Signed-off-by: Colin MacKenzie <[email protected]>
1 parent dc7e95a commit b9ffd72

File tree

3 files changed

+28
-6
lines changed

3 files changed

+28
-6
lines changed

rclcpp/include/rclcpp/service.hpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ class Service : public ServiceBase
180180
std::weak_ptr<rcl_node_t> weak_node_handle(node_handle_);
181181
// rcl does the static memory allocation here
182182
service_handle_ = std::shared_ptr<rcl_service_t>(
183-
new rcl_service_t, [weak_node_handle](rcl_service_t * service)
183+
new rcl_service_t, [weak_node_handle, service_name](rcl_service_t * service)
184184
{
185185
auto handle = weak_node_handle.lock();
186186
if (handle) {
@@ -192,10 +192,10 @@ class Service : public ServiceBase
192192
rcl_reset_error();
193193
}
194194
} else {
195-
RCLCPP_ERROR(
195+
RCLCPP_ERROR_STREAM(
196196
rclcpp::get_logger("rclcpp"),
197-
"Error in destruction of rcl service handle: "
198-
"the Node Handle was destructed too early. You will leak memory");
197+
"Error in destruction of rcl service handle " << service_name <<
198+
": the Node Handle was destructed too early. You will leak memory");
199199
}
200200
delete service;
201201
});

rclcpp/src/rclcpp/node.cpp

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,18 @@ Node::Node(
185185
}
186186

187187
Node::~Node()
188-
{}
188+
{
189+
// release sub-interfaces in an order that allows them to consult with node_base during tear-down
190+
node_waitables_.reset();
191+
node_time_source_.reset();
192+
node_parameters_.reset();
193+
node_clock_.reset();
194+
node_services_.reset();
195+
node_topics_.reset();
196+
node_timers_.reset();
197+
node_logging_.reset();
198+
node_graph_.reset();
199+
}
189200

190201
const char *
191202
Node::get_name() const

rclcpp_lifecycle/src/lifecycle_node.cpp

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,18 @@ LifecycleNode::LifecycleNode(
128128
}
129129

130130
LifecycleNode::~LifecycleNode()
131-
{}
131+
{
132+
// release sub-interfaces in an order that allows them to consult with node_base during tear-down
133+
node_waitables_.reset();
134+
node_time_source_.reset();
135+
node_parameters_.reset();
136+
node_clock_.reset();
137+
node_services_.reset();
138+
node_topics_.reset();
139+
node_timers_.reset();
140+
node_logging_.reset();
141+
node_graph_.reset();
142+
}
132143

133144
const char *
134145
LifecycleNode::get_name() const

0 commit comments

Comments
 (0)