From 5e565c7e75e6be9dbea7a4ee7f30cdd8c840d35b Mon Sep 17 00:00:00 2001
From: Tully Foote <tfoote@osrfoundation.org>
Date: Tue, 21 Nov 2017 11:42:44 -0800
Subject: [PATCH] detach nodes from executors in destruction. (#404)

---
 rclcpp/CMakeLists.txt          |  8 +++++
 rclcpp/src/rclcpp/executor.cpp | 10 ++++++
 rclcpp/test/test_executor.cpp  | 62 ++++++++++++++++++++++++++++++++++
 3 files changed, 80 insertions(+)
 create mode 100644 rclcpp/test/test_executor.cpp

diff --git a/rclcpp/CMakeLists.txt b/rclcpp/CMakeLists.txt
index fcce5dd..54235f0 100644
--- a/rclcpp/CMakeLists.txt
+++ b/rclcpp/CMakeLists.txt
@@ -277,6 +277,14 @@ if(BUILD_TESTING)
     target_link_libraries(test_duration ${PROJECT_NAME})
   endif()
 
+  ament_add_gtest(test_executor test/test_executor.cpp
+    APPEND_LIBRARY_DIRS "${append_library_dirs}")
+  if(TARGET test_executor)
+    ament_target_dependencies(test_executor
+      "rcl")
+    target_link_libraries(test_executor ${PROJECT_NAME})
+  endif()
+
   ament_add_gmock(test_logging test/test_logging.cpp)
   target_link_libraries(test_logging ${PROJECT_NAME})
 
diff --git a/rclcpp/src/rclcpp/executor.cpp b/rclcpp/src/rclcpp/executor.cpp
index 4fbe867..f2cc48d 100644
--- a/rclcpp/src/rclcpp/executor.cpp
+++ b/rclcpp/src/rclcpp/executor.cpp
@@ -73,6 +73,16 @@ Executor::Executor(const ExecutorArgs & args)
 
 Executor::~Executor()
 {
+  // Disassocate all nodes
+  for (auto & weak_node : weak_nodes_) {
+    auto node = weak_node.lock();
+    if (node) {
+      std::atomic_bool & has_executor = node->get_associated_with_executor_atomic();
+      has_executor.store(false);
+    }
+  }
+  weak_nodes_.clear();
+
   // Finalize the waitset.
   if (rcl_wait_set_fini(&waitset_) != RCL_RET_OK) {
     fprintf(stderr,
diff --git a/rclcpp/test/test_executor.cpp b/rclcpp/test/test_executor.cpp
new file mode 100644
index 0000000..f57b961
--- /dev/null
+++ b/rclcpp/test/test_executor.cpp
@@ -0,0 +1,62 @@
+// Copyright 2017 Open Source Robotics Foundation, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <gtest/gtest.h>
+
+#include <algorithm>
+#include <chrono>
+#include <limits>
+#include <memory>
+#include <string>
+
+#include "rcl/error_handling.h"
+#include "rcl/time.h"
+#include "rclcpp/clock.hpp"
+#include "rclcpp/duration.hpp"
+#include "rclcpp/rclcpp.hpp"
+
+using namespace std::chrono_literals;
+
+class TestExcutors : public ::testing::Test
+{
+protected:
+  static void SetUpTestCase()
+  {
+    rclcpp::init(0, nullptr);
+  }
+
+  void SetUp()
+  {
+    node = std::make_shared<rclcpp::node::Node>("my_node");
+  }
+
+  void TearDown()
+  {
+    node.reset();
+  }
+
+  rclcpp::node::Node::SharedPtr node;
+};
+
+// Make sure that executors detach from nodes when destructing
+TEST_F(TestExcutors, detachOnDestruction) {
+  {
+    rclcpp::executors::SingleThreadedExecutor executor;
+    executor.add_node(node);
+  }
+  {
+    rclcpp::executors::SingleThreadedExecutor executor;
+    EXPECT_NO_THROW(executor.add_node(node));
+  }
+}