diff --git a/rclcpp/CMakeLists.txt b/rclcpp/CMakeLists.txt index 6cf748a..f2aa4eb 100644 --- a/rclcpp/CMakeLists.txt +++ b/rclcpp/CMakeLists.txt @@ -120,6 +120,13 @@ if(BUILD_TESTING) ${PROJECT_NAME} ) endif() + ament_add_gtest(test_find_weak_nodes test/test_find_weak_nodes.cpp) + if(TARGET test_find_weak_nodes) + target_include_directories(test_find_weak_nodes PUBLIC + ${rcl_INCLUDE_DIRS} + ) + target_link_libraries(test_find_weak_nodes ${PROJECT_NAME}) + endif() endif() ament_package( diff --git a/rclcpp/include/rclcpp/strategies/allocator_memory_strategy.hpp b/rclcpp/include/rclcpp/strategies/allocator_memory_strategy.hpp index 1d36029..39bdde3 100644 --- a/rclcpp/include/rclcpp/strategies/allocator_memory_strategy.hpp +++ b/rclcpp/include/rclcpp/strategies/allocator_memory_strategy.hpp @@ -144,7 +144,7 @@ public: for (auto & weak_node : weak_nodes) { auto node = weak_node.lock(); if (!node) { - has_invalid_weak_nodes = false; + has_invalid_weak_nodes = true; continue; } for (auto & weak_group : node->get_callback_groups()) { diff --git a/rclcpp/test/test_find_weak_nodes.cpp b/rclcpp/test/test_find_weak_nodes.cpp new file mode 100644 index 0000000..e085076 --- /dev/null +++ b/rclcpp/test/test_find_weak_nodes.cpp @@ -0,0 +1,76 @@ +// Copyright 2016 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 +#include + +#include "rclcpp/strategies/allocator_memory_strategy.hpp" +#include "rclcpp/memory_strategy.hpp" +#include "rclcpp/node.hpp" +#include "rclcpp/rclcpp.hpp" + +class TestFindWeakNodes : public ::testing::Test +{ +protected: + static void SetUpTestCase() + { + rclcpp::init(0, nullptr); + } +}; + +TEST_F(TestFindWeakNodes, allocator_strategy_with_weak_nodes) { + // GIVEN + // A vector of weak pointers to nodes + auto memory_strategy = std::make_shared< + rclcpp::memory_strategies::allocator_memory_strategy::AllocatorMemoryStrategy<>>(); + auto existing_node = rclcpp::node::Node::make_shared("existing_node"); + auto dead_node = rclcpp::node::Node::make_shared("dead_node"); + rclcpp::memory_strategy::MemoryStrategy::WeakNodeVector weak_nodes; + weak_nodes.push_back(existing_node); + weak_nodes.push_back(dead_node); + + // AND + // Delete dead_node, creating a dangling pointer in weak_nodes + dead_node.reset(); + ASSERT_FALSE(weak_nodes[0].expired()); + ASSERT_TRUE(weak_nodes[1].expired()); + + // WHEN + bool has_invalid_weak_nodes = memory_strategy->collect_entities(weak_nodes); + + // THEN + // The result of finding dangling node pointers should be true + ASSERT_TRUE(has_invalid_weak_nodes); +} + +TEST_F(TestFindWeakNodes, allocator_strategy_no_weak_nodes) { + // GIVEN + // A vector of weak pointers to nodes, all valid + auto memory_strategy = std::make_shared< + rclcpp::memory_strategies::allocator_memory_strategy::AllocatorMemoryStrategy<>>(); + auto existing_node1 = rclcpp::node::Node::make_shared("existing_node1"); + auto existing_node2 = rclcpp::node::Node::make_shared("existing_node2"); + rclcpp::memory_strategy::MemoryStrategy::WeakNodeVector weak_nodes; + weak_nodes.push_back(existing_node1); + weak_nodes.push_back(existing_node2); + ASSERT_FALSE(weak_nodes[0].expired()); + ASSERT_FALSE(weak_nodes[1].expired()); + + // WHEN + bool has_invalid_weak_nodes = memory_strategy->collect_entities(weak_nodes); + + // THEN + // The result of finding dangling node pointers should be false + ASSERT_FALSE(has_invalid_weak_nodes); +}