Skip to content

Commit 2fb0a51

Browse files
dv-picknikclaude
andcommitted
add Finally control node for guaranteed cleanup
Finally ticks its first child (main), then ticks its second child (cleanup) after main returns SUCCESS, FAILURE or SKIPPED, or throws. If main threw, the exception is rethrown once cleanup finishes. Otherwise the node returns main's status, or FAILURE if cleanup failed. An exception from cleanup resets the node and propagates. When halted while main or cleanup is RUNNING, it halts main and keeps ticking cleanup every 10 ms until cleanup finishes or the halt_timeout_msec port (default 10000) runs out, so asynchronous cleanup completes. halt() never throws, because it also runs from ~Tree(), so it prints problems to stderr instead. ControlNode::haltChild now resets the child's status before rethrowing an exception from its halt(), so a later reset does not halt that child a second time. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 6a3b337 commit 2fb0a51

9 files changed

Lines changed: 897 additions & 1 deletion

File tree

‎CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,7 @@ list(APPEND BT_SOURCE
182182
src/controls/sequence_node.cpp
183183
src/controls/sequence_with_memory_node.cpp
184184
src/controls/switch_node.cpp
185+
src/controls/finally_node.cpp
185186
src/controls/try_catch_node.cpp
186187
src/controls/while_do_else_node.cpp
187188

‎include/behaviortree_cpp/behavior_tree.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include "behaviortree_cpp/actions/updated_action.h"
2626
#include "behaviortree_cpp/condition_node.h"
2727
#include "behaviortree_cpp/controls/fallback_node.h"
28+
#include "behaviortree_cpp/controls/finally_node.h"
2829
#include "behaviortree_cpp/controls/if_then_else_node.h"
2930
#include "behaviortree_cpp/controls/parallel_all_node.h"
3031
#include "behaviortree_cpp/controls/parallel_node.h"
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
#pragma once
2+
3+
#include "behaviortree_cpp/control_node.h"
4+
5+
#include <exception>
6+
7+
namespace BT
8+
{
9+
/**
10+
* @brief The Finally node ticks its first child ("main") and then ticks its
11+
* second child ("cleanup"), like try/finally.
12+
*
13+
* - Cleanup runs after main returns SUCCESS, FAILURE or SKIPPED.
14+
* - If main throws (any type), main is halted, cleanup runs, and then the
15+
* exception is rethrown.
16+
* - The node returns main's status, or FAILURE if cleanup fails.
17+
* - If this node is halted while main or cleanup is RUNNING, main is halted
18+
* and cleanup is ticked again every 10 ms on the thread calling halt(), so
19+
* asynchronous cleanup can finish. This stops when cleanup finishes, fails
20+
* or throws, or when "halt_timeout_msec" runs out, in which case cleanup is
21+
* halted unfinished. Cleanup always gets at least one tick, no later tick
22+
* starts after the timeout, and a tick that blocks is not cut short. halt() blocks its caller, and any lock the
23+
* caller holds, until then, so a slow cleanup also delays a parent such as
24+
* a ReactiveSequence. Call halt() from the thread that ticks the tree.
25+
* - An exception thrown by cleanup in tick() halts cleanup, leaves the node
26+
* IDLE and propagates. It replaces main's status, or main's exception,
27+
* which is dropped. The next tick starts with main.
28+
* - halt() never throws, because it also runs from ~Tree(). It prints to
29+
* stderr when cleanup throws, fails or times out, and when halting a child
30+
* throws. A pending exception from main is dropped by a halt.
31+
*
32+
* Requires exactly 2 children, checked when the XML is loaded and on tick.
33+
*/
34+
class FinallyNode : public ControlNode
35+
{
36+
public:
37+
FinallyNode(const std::string& name, const NodeConfig& config);
38+
39+
~FinallyNode() override = default;
40+
41+
FinallyNode(const FinallyNode&) = delete;
42+
FinallyNode& operator=(const FinallyNode&) = delete;
43+
FinallyNode(FinallyNode&&) = delete;
44+
FinallyNode& operator=(FinallyNode&&) = delete;
45+
46+
static PortsList providedPorts()
47+
{
48+
return { InputPort<unsigned>("halt_timeout_msec", kDefaultHaltTimeoutMsec,
49+
"When halted, how long to keep ticking cleanup "
50+
"before halting it unfinished, in milliseconds") };
51+
}
52+
53+
void halt() override;
54+
55+
private:
56+
static constexpr unsigned kDefaultHaltTimeoutMsec = 10000;
57+
58+
bool in_cleanup_ = false;
59+
NodeStatus main_status_ = NodeStatus::IDLE;
60+
std::exception_ptr main_exception_;
61+
62+
void haltChildNoThrow(size_t i);
63+
void finishCleanupDuringHalt();
64+
65+
BT::NodeStatus tick() override;
66+
};
67+
68+
} // namespace BT

‎src/bt_factory.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ BehaviorTreeFactory::BehaviorTreeFactory() : _p(new PImpl)
132132
registerNodeType<IfThenElseNode>("IfThenElse");
133133
registerNodeType<WhileDoElseNode>("WhileDoElse");
134134
registerNodeType<TryCatchNode>("TryCatch");
135+
registerNodeType<FinallyNode>("Finally");
135136

136137
registerNodeType<InverterNode>("Inverter");
137138

‎src/control_node.cpp‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,16 @@ void ControlNode::haltChild(size_t i)
5757
auto* child = children_nodes_[i];
5858
if(child->status() == NodeStatus::RUNNING)
5959
{
60-
child->haltNode();
60+
try
61+
{
62+
child->haltNode();
63+
}
64+
catch(...)
65+
{
66+
// Don't leave the child RUNNING, or the next reset would halt it again.
67+
child->resetStatus();
68+
throw;
69+
}
6170
}
6271
child->resetStatus();
6372
}

‎src/controls/finally_node.cpp‎

Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,203 @@
1+
#include "behaviortree_cpp/controls/finally_node.h"
2+
3+
#include <chrono>
4+
#include <exception>
5+
#include <iostream>
6+
#include <string_view>
7+
#include <thread>
8+
#include <utility>
9+
10+
namespace BT
11+
{
12+
namespace
13+
{
14+
constexpr std::chrono::milliseconds kHaltTickPeriod{ 10 };
15+
16+
void printException(std::string_view node_name, const char* context,
17+
const std::exception_ptr& exception)
18+
{
19+
std::cerr << "[" << node_name << "]: " << context << ": ";
20+
try
21+
{
22+
std::rethrow_exception(exception);
23+
}
24+
catch(const std::exception& ex)
25+
{
26+
std::cerr << ex.what() << std::endl;
27+
}
28+
catch(...)
29+
{
30+
std::cerr << "non-std exception" << std::endl;
31+
}
32+
}
33+
} // namespace
34+
35+
FinallyNode::FinallyNode(const std::string& name, const NodeConfig& config)
36+
: ControlNode::ControlNode(name, config)
37+
{
38+
setRegistrationID("Finally");
39+
}
40+
41+
void FinallyNode::halt()
42+
{
43+
// halt() also runs from ~Tree(), where a propagating exception terminates, so nothing here throws.
44+
if(status() == NodeStatus::RUNNING && children_nodes_.size() == 2)
45+
{
46+
if(!in_cleanup_)
47+
{
48+
haltChildNoThrow(0);
49+
}
50+
finishCleanupDuringHalt();
51+
}
52+
for(size_t i = 0; i < children_nodes_.size(); i++)
53+
{
54+
haltChildNoThrow(i);
55+
}
56+
in_cleanup_ = false;
57+
main_status_ = NodeStatus::IDLE;
58+
main_exception_ = nullptr;
59+
resetStatus();
60+
}
61+
62+
void FinallyNode::finishCleanupDuringHalt()
63+
{
64+
unsigned timeout_msec = kDefaultHaltTimeoutMsec;
65+
if(auto input = getInput<unsigned>("halt_timeout_msec"))
66+
{
67+
timeout_msec = input.value();
68+
}
69+
else
70+
{
71+
std::cerr << "[" << name() << "]: cannot read halt_timeout_msec (" << input.error()
72+
<< "), using " << kDefaultHaltTimeoutMsec << " ms" << std::endl;
73+
}
74+
const auto deadline =
75+
std::chrono::steady_clock::now() + std::chrono::milliseconds(timeout_msec);
76+
try
77+
{
78+
// Cleanup is commonly asynchronous, so keep ticking it until it finishes rather than
79+
// halting it after its first tick.
80+
NodeStatus cleanup_status = children_nodes_[1]->executeTick();
81+
while(cleanup_status == NodeStatus::RUNNING)
82+
{
83+
// Start no tick after the deadline, so a tick that blocks cannot extend the wait.
84+
const auto next_tick = std::chrono::steady_clock::now() + kHaltTickPeriod;
85+
if(next_tick > deadline)
86+
{
87+
std::cerr << "[" << name()
88+
<< "]: cleanup did not finish within halt_timeout_msec ("
89+
<< timeout_msec << " ms), halting it unfinished" << std::endl;
90+
return;
91+
}
92+
std::this_thread::sleep_until(next_tick);
93+
cleanup_status = children_nodes_[1]->executeTick();
94+
}
95+
if(cleanup_status == NodeStatus::FAILURE)
96+
{
97+
std::cerr << "[" << name() << "]: cleanup returned FAILURE during halt"
98+
<< std::endl;
99+
}
100+
}
101+
catch(...)
102+
{
103+
printException(name(), "cleanup threw during halt", std::current_exception());
104+
}
105+
}
106+
107+
void FinallyNode::haltChildNoThrow(size_t i)
108+
{
109+
try
110+
{
111+
haltChild(i);
112+
}
113+
catch(...)
114+
{
115+
printException(name(), "a child threw while being halted", std::current_exception());
116+
}
117+
}
118+
119+
NodeStatus FinallyNode::tick()
120+
{
121+
if(children_nodes_.size() != 2)
122+
{
123+
throw LogicError("[", name(), "]: Finally requires exactly 2 children");
124+
}
125+
126+
if(!isStatusActive(status()))
127+
{
128+
in_cleanup_ = false;
129+
main_exception_ = nullptr;
130+
}
131+
132+
setStatus(NodeStatus::RUNNING);
133+
134+
if(!in_cleanup_)
135+
{
136+
try
137+
{
138+
main_status_ = children_nodes_[0]->executeTick();
139+
}
140+
catch(...)
141+
{
142+
main_exception_ = std::current_exception();
143+
haltChildNoThrow(0);
144+
main_status_ = NodeStatus::FAILURE;
145+
}
146+
147+
if(main_status_ == NodeStatus::RUNNING)
148+
{
149+
return NodeStatus::RUNNING;
150+
}
151+
if(main_status_ == NodeStatus::IDLE)
152+
{
153+
throw LogicError("[", name(), "]: A child should not return IDLE");
154+
}
155+
in_cleanup_ = true;
156+
}
157+
158+
NodeStatus cleanup_status = NodeStatus::IDLE;
159+
try
160+
{
161+
cleanup_status = children_nodes_[1]->executeTick();
162+
}
163+
catch(...)
164+
{
165+
// As in a finally block, the exception replaces main's outcome and ends this run, so a later
166+
// halt, such as the one ~Tree() runs, does not retry cleanup.
167+
// Main finished but was not reset yet. haltChildNoThrow() resets both without throwing, so the
168+
// cleanup exception is the one that propagates.
169+
for(size_t i = 0; i < children_nodes_.size(); i++)
170+
{
171+
haltChildNoThrow(i);
172+
}
173+
in_cleanup_ = false;
174+
main_exception_ = nullptr;
175+
resetStatus();
176+
throw;
177+
}
178+
if(cleanup_status == NodeStatus::RUNNING)
179+
{
180+
return NodeStatus::RUNNING;
181+
}
182+
183+
resetChildren();
184+
in_cleanup_ = false;
185+
if(main_exception_)
186+
{
187+
// executeTick() keeps our RUNNING status when tick() throws, and halt() would rerun cleanup.
188+
resetStatus();
189+
std::rethrow_exception(std::exchange(main_exception_, nullptr));
190+
}
191+
if(cleanup_status == NodeStatus::FAILURE)
192+
{
193+
return NodeStatus::FAILURE;
194+
}
195+
if(main_status_ == NodeStatus::SKIPPED)
196+
{
197+
// executeTick() keeps our RUNNING status on SKIPPED, and halt() would rerun cleanup.
198+
resetStatus();
199+
}
200+
return main_status_;
201+
}
202+
203+
} // namespace BT

‎src/xml_parsing.cpp‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -642,6 +642,11 @@ void VerifyXML(const std::string& xml_text,
642642
ThrowError(line_number, std::string("The node 'TryCatch' must have "
643643
"at least 2 children"));
644644
}
645+
if(registered_name == "Finally" && children_count != 2)
646+
{
647+
ThrowError(line_number, std::string("The node 'Finally' must have "
648+
"exactly 2 children"));
649+
}
645650
if(registered_name == "ReactiveSequence")
646651
{
647652
size_t async_count = 0;

‎tests/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ set(BT_TESTS
4747
gtest_switch.cpp
4848
gtest_tree.cpp
4949
gtest_try_catch.cpp
50+
gtest_finally.cpp
5051
gtest_exception_tracking.cpp
5152
gtest_updates.cpp
5253
gtest_wakeup.cpp

0 commit comments

Comments
 (0)