diff --git a/src/tree_node.cpp b/src/tree_node.cpp index b0c612f7d..00eb1f595 100644 --- a/src/tree_node.cpp +++ b/src/tree_node.cpp @@ -158,7 +158,17 @@ NodeStatus TreeNode::executeTick() void TreeNode::haltNode() { - halt(); + try + { + halt(); + } + catch(...) + { + // Don't leave the node RUNNING, or the next reset would halt it again + // (and throw again, possibly from a destructor). + resetStatus(); + throw; + } const auto& parse_executor = _p->post_parsed[size_t(PostCond::ON_HALTED)]; if(parse_executor) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index ad39d7879..103715ee6 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -49,6 +49,7 @@ set(BT_TESTS gtest_switch.cpp gtest_tree.cpp gtest_try_catch.cpp + gtest_halt_child.cpp gtest_exception_tracking.cpp gtest_updates.cpp gtest_wakeup.cpp diff --git a/tests/gtest_halt_child.cpp b/tests/gtest_halt_child.cpp new file mode 100644 index 000000000..2e9f4ff1c --- /dev/null +++ b/tests/gtest_halt_child.cpp @@ -0,0 +1,122 @@ +#include "behaviortree_cpp/bt_factory.h" +#include "behaviortree_cpp/control_node.h" + +#include + +#include + +using BT::NodeStatus; + +namespace +{ +// RUNNING until halted, and its onHalted() throws. Counts the halts. +class ThrowOnHalt : public BT::StatefulActionNode +{ +public: + ThrowOnHalt(const std::string& name, const BT::NodeConfig& config, int* halts) + : StatefulActionNode(name, config), halts_(halts) + {} + static BT::PortsList providedPorts() + { + return {}; + } + NodeStatus onStart() override + { + return NodeStatus::RUNNING; + } + NodeStatus onRunning() override + { + return NodeStatus::RUNNING; + } + void onHalted() override + { + (*halts_)++; + throw std::runtime_error("halt failed"); + } + +private: + int* halts_; +}; + +// Ticks its only child and halts it on request through haltChild(). +class HaltsChild : public BT::ControlNode +{ +public: + HaltsChild(const std::string& name, const BT::NodeConfig& config) + : ControlNode(name, config) + {} + static BT::PortsList providedPorts() + { + return {}; + } + NodeStatus tick() override + { + setStatus(NodeStatus::RUNNING); + return children_nodes_[0]->executeTick(); + } + void halt() override + { + haltChild(0); + resetStatus(); + } +}; +} // namespace + +// A child whose halt() throws must not stay RUNNING, or the next haltChild() +// halts it again and throws a second time. +TEST(ControlNodeHaltChild, ThrowingHaltIsNotRepeated) +{ + int halts = 0; + BT::BehaviorTreeFactory factory; + factory.registerNodeType("ThrowOnHalt", &halts); + factory.registerNodeType("HaltsChild"); + auto tree = factory.createTreeFromText(R"( + + + )"); + + EXPECT_EQ(tree.tickOnce(), NodeStatus::RUNNING); + auto* node = dynamic_cast(tree.rootNode()); + EXPECT_THROW(node->halt(), std::runtime_error); + EXPECT_EQ(halts, 1); + EXPECT_NO_THROW(node->halt()); + EXPECT_EQ(halts, 1); + EXPECT_EQ(tree.subtrees[0]->nodes[1]->status(), NodeStatus::IDLE); +} + +namespace +{ +// Built-in parents halt their children via resetChildren()/resetChild(), not +// haltChild(): the failed child must end up IDLE there too. +void checkBuiltinParentThrowingHalt(const char* body) +{ + int halts = 0; + BT::BehaviorTreeFactory factory; + factory.registerNodeType("ThrowOnHalt", &halts); + auto tree = + factory.createTreeFromText(std::string(R"()") + + body + ""); + + EXPECT_EQ(tree.tickOnce(), NodeStatus::RUNNING); + EXPECT_ANY_THROW(tree.haltTree()); + EXPECT_EQ(halts, 1); + for(const auto& node : tree.subtrees[0]->nodes) + { + EXPECT_EQ(node->status(), NodeStatus::IDLE) << node->name(); + } + EXPECT_NO_THROW(tree.haltTree()); + EXPECT_EQ(halts, 1); + // ~Tree() calls haltTree() again: it must not throw (-> terminate). +} +} // namespace + +TEST(ControlNodeHaltChild, ThrowingHaltInSequence) +{ + checkBuiltinParentThrowingHalt(""); +} + +TEST(ControlNodeHaltChild, ThrowingHaltInRetryDecorator) +{ + checkBuiltinParentThrowingHalt( + R"()"); +}