From 92fe787a74927387a76128d511ab5b11ff38de46 Mon Sep 17 00:00:00 2001
From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com>
Date: Tue, 3 Aug 2021 17:03:02 -0300
Subject: [PATCH] Fix SEGV caused by order of destruction of Node
 sub-interfaces (#1469) (#1736)

* 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>
---
 rclcpp/include/rclcpp/service.hpp       |  8 ++++----
 rclcpp/src/rclcpp/node.cpp              | 13 ++++++++++++-
 rclcpp_lifecycle/src/lifecycle_node.cpp | 13 ++++++++++++-
 3 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/rclcpp/include/rclcpp/service.hpp b/rclcpp/include/rclcpp/service.hpp
index 094b628..509efe8 100644
--- a/rclcpp/include/rclcpp/service.hpp
+++ b/rclcpp/include/rclcpp/service.hpp
@@ -180,7 +180,7 @@ public:
     std::weak_ptr<rcl_node_t> weak_node_handle(node_handle_);
     // rcl does the static memory allocation here
     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();
         if (handle) {
@@ -192,10 +192,10 @@ public:
             rcl_reset_error();
           }
         } else {
-          RCLCPP_ERROR(
+          RCLCPP_ERROR_STREAM(
             rclcpp::get_logger("rclcpp"),
-            "Error in destruction of rcl service handle: "
-            "the Node Handle was destructed too early. You will leak memory");
+            "Error in destruction of rcl service handle " << service_name <<
+              ": the Node Handle was destructed too early. You will leak memory");
         }
         delete service;
       });
diff --git a/rclcpp/src/rclcpp/node.cpp b/rclcpp/src/rclcpp/node.cpp
index 778d33e..ba63ce6 100644
--- a/rclcpp/src/rclcpp/node.cpp
+++ b/rclcpp/src/rclcpp/node.cpp
@@ -184,7 +184,18 @@ 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 *
 Node::get_name() const
diff --git a/rclcpp_lifecycle/src/lifecycle_node.cpp b/rclcpp_lifecycle/src/lifecycle_node.cpp
index 1d72105..8341b1c 100644
--- a/rclcpp_lifecycle/src/lifecycle_node.cpp
+++ b/rclcpp_lifecycle/src/lifecycle_node.cpp
@@ -126,7 +126,18 @@ 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 *
 LifecycleNode::get_name() const