Skip to content

Commit a2f6a86

Browse files
facontidavideclaude
facontidavide
and
claude
committed
Fix #969: LoopNode accepts std::vector<T> in addition to SharedQueue<T>
LoopNode's queue port now uses an untyped BidirectionalPort to avoid port type mismatch when upstream nodes produce std::vector<T>. At runtime, tick() tries SharedQueue<T> first, then falls back to std::vector<T> (converted to deque on first read), then string (via convertFromString). This allows upstream nodes that output vector<T> to feed directly into LoopNode without manual conversion. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent a0286a0 commit a2f6a86

File tree

3 files changed

+349
-3
lines changed

3 files changed

+349
-3
lines changed

‎FIXME.md‎

Lines changed: 259 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,259 @@
1+
# FIXME - Bug Tracker
2+
3+
This file tracks reported bugs from GitHub issues. For each bug, the workflow is:
4+
1. Write a unit test that reproduces the issue
5+
2. Evaluate whether the behavior diverges from intended library semantics
6+
3. Fix the issue (if confirmed as a bug)
7+
8+
---
9+
10+
## Scripting / Script Parser
11+
12+
### [x] #1029 - Mathematical order of operations in script is wrong
13+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/1029
14+
- **Summary:** `A - B + C` is evaluated as `A - (B + C)` instead of left-to-right. The expression parser applies incorrect associativity for addition and subtraction at the same precedence level.
15+
- **Component:** `src/scripting/`
16+
- **Test file:** `tests/gtest_scripting.cpp` (or new)
17+
18+
### [x] #923 - Out-of-bounds read in script validation
19+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/923
20+
- **Summary:** `ValidateScript` can create a `std::string` from a non-NULL-terminated fixed-size stack buffer when parsing invalid scripts that generate large error messages, causing `strlen` to read out of bounds.
21+
- **Component:** `src/scripting/`
22+
- **Test file:** `tests/gtest_scripting.cpp` (or new)
23+
24+
### [x] #832 - Script compare with negative number throws exception
25+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/832
26+
- **Summary:** Using a negative number in a script precondition (e.g., `_failureIf="A!=-1"`) throws a runtime exception. The scripting parser cannot handle negative number literals in comparisons.
27+
- **Component:** `src/scripting/`
28+
- **Test file:** `tests/gtest_scripting.cpp` (or new)
29+
30+
---
31+
32+
## Ports / Type Conversion
33+
34+
### [x] #1065 - Type conversion problem with subtree
35+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/1065
36+
- **Summary:** Passing a `std::deque<double>` through a SubTree port via a string literal (e.g., `"1;2;3"`) throws `std::runtime_error` about no safe conversion between `std::string` and `std::shared_ptr<std::deque<double>>`. String-to-deque conversion fails at subtree boundary.
37+
- **Component:** `src/xml_parsing.cpp`, `include/behaviortree_cpp/basic_types.h`
38+
- **Test file:** `tests/gtest_ports.cpp` (or new)
39+
40+
### [x] #982 - Vectors initialized with `json:[]` string instead of empty
41+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/982
42+
- **Summary:** When a port of type `std::vector<std::string>` has a default empty value `{}` and no input is specified in XML, the vector is initialized with the literal string `"json:[]"` instead of being an empty vector.
43+
- **Component:** `include/behaviortree_cpp/basic_types.h`, `src/xml_parsing.cpp`
44+
- **Test file:** `tests/gtest_ports.cpp`
45+
46+
### [x] #969 - LoopNode: std::vector<T> vs std::deque<T> type mismatch
47+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/969
48+
- **Summary:** `LoopNode<T>` uses `std::shared_ptr<std::deque<T>>` for its queue port, but upstream nodes often produce `std::vector<T>`. This creates a port type conflict during tree creation.
49+
- **Component:** `include/behaviortree_cpp/decorators/loop_node.h`
50+
- **Test file:** `tests/gtest_decorator.cpp` (or new)
51+
52+
### [ ] #948 - ParseString errors with custom enum types
53+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/948
54+
- **Summary:** `parseString` in `tree_node.h` has an unsafe `find` on an unordered_map that may not be instantiated yet if `getInput` is called during construction. Also, custom enum types cannot be properly parsed via `convertFromString`.
55+
- **Component:** `include/behaviortree_cpp/tree_node.h`, `include/behaviortree_cpp/basic_types.h`
56+
- **Test file:** `tests/gtest_ports.cpp`
57+
58+
### [ ] #858 - getInput throws when default parameter is passed
59+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/858
60+
- **Summary:** Calling `getInput` with a default parameter value throws `std::out_of_range` (`_Map_base::at`). The internal map lookup fails when the port has a default value but the entry does not exist in the expected location.
61+
- **Component:** `include/behaviortree_cpp/tree_node.h`
62+
- **Test file:** `tests/gtest_ports.cpp`
63+
64+
---
65+
66+
## Blackboard
67+
68+
### [ ] #974 - Blackboard set() stores values as string type
69+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/974
70+
- **Summary:** When using `blackboard->set(key, value.dump())` to set numeric values from JSON, the blackboard stores them as strings. Subsequent scripting operations with numeric operators fail because string types cannot be compared numerically.
71+
- **Component:** `src/blackboard.cpp`, `include/behaviortree_cpp/blackboard.h`
72+
- **Test file:** `tests/gtest_blackboard.cpp`
73+
74+
### [ ] #942 - Default blackboard entry not created for locked content
75+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/942
76+
- **Summary:** When using `getLockedPortContent` on a port with a default blackboard entry, no blackboard entry is actually created on the first call. The returned `AnyPtrLocked` is a default/empty value not placed into the blackboard.
77+
- **Component:** `src/blackboard.cpp`, `include/behaviortree_cpp/blackboard.h`
78+
- **Test file:** `tests/gtest_blackboard.cpp`
79+
80+
### [ ] #408 - debugMessage does not print parent values in SubTree
81+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/408
82+
- **Summary:** `config().blackboard->debugMessage()` inside a SubTree node's constructor does not show remapped entries from the parent blackboard. The function only checks `internal_to_external_` after finding an entry in `storage_`.
83+
- **Component:** `src/blackboard.cpp`
84+
- **Test file:** `tests/gtest_blackboard.cpp`
85+
86+
---
87+
88+
## Control Nodes (Parallel)
89+
90+
### [ ] #1045 - Test PauseWithRetry is flaky
91+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/1045
92+
- **Summary:** The `Parallel.PauseWithRetry` test fails intermittently due to tight timing assertions. Measured time drifts beyond the allowed margin.
93+
- **Component:** `tests/gtest_parallel.cpp`
94+
- **Test file:** `tests/gtest_parallel.cpp`
95+
96+
### [ ] #1041 - Performance degradation after prolonged operation in parallel nodes
97+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/1041
98+
- **Summary:** After prolonged operation, when a parallel node's child detects an exception and exits, the interval before other nodes wait for execution becomes increasingly longer. Likely related to resource accumulation or thread management.
99+
- **Component:** `src/controls/parallel_node.cpp`
100+
- **Test file:** `tests/gtest_parallel.cpp` (or new)
101+
102+
### [ ] #966 - ParallelAll max_failures off-by-one semantics
103+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/966
104+
- **Summary:** `ParallelAll` documents `max_failures` as "if the number of children returning FAILURE *exceeds* this value" but implements `>=` (greater-than-or-equal). Documentation and implementation disagree.
105+
- **Component:** `src/controls/parallel_all_node.cpp`
106+
- **Test file:** `tests/gtest_parallel.cpp`
107+
108+
---
109+
110+
## Control Nodes (Reactive)
111+
112+
### [ ] #1031 - Race condition in ReactiveSequence/ReactiveFallback with async children
113+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/1031
114+
- **Summary:** With backchaining patterns, a previously `RUNNING` child is halted only *after* another child's `onStart` is executed. Two children are in `RUNNING` state simultaneously briefly, causing race conditions.
115+
- **Component:** `src/controls/reactive_sequence.cpp`, `src/controls/reactive_fallback.cpp`
116+
- **Test file:** `tests/gtest_reactive.cpp` (or new)
117+
118+
### [ ] #917 - Precondition not re-checked in ReactiveSequence
119+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/917
120+
- **Summary:** When a node has a `_successIf` precondition inside a `ReactiveSequence`, the precondition is never re-evaluated after the node enters `RUNNING` state. The node continues returning `RUNNING` instead of `SUCCESS`.
121+
- **Component:** `src/controls/reactive_sequence.cpp`, `src/tree_node.cpp`
122+
- **Test file:** `tests/gtest_reactive.cpp`
123+
124+
---
125+
126+
## XML Parsing
127+
128+
### [ ] #979 - Recursive behavior trees cause stack overflow
129+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/979
130+
- **Summary:** A behavior tree can reference its own ID as a SubTree, causing infinite recursion and stack overflow during XML parsing. The parser does not detect or prevent cyclic subtree references.
131+
- **Component:** `src/xml_parsing.cpp`
132+
- **Test file:** `tests/gtest_factory.cpp` (or new)
133+
134+
### [ ] #883 - JSON string in port value interpreted as blackboard reference
135+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/883
136+
- **Summary:** Setting a port value to a JSON string in XML (e.g., `settings='{{"a": 0.8}}'`) causes `{}` content to be interpreted as blackboard variable references instead of literal JSON.
137+
- **Component:** `src/xml_parsing.cpp`, `src/tree_node.cpp`
138+
- **Test file:** `tests/gtest_factory.cpp` (or new)
139+
140+
### [ ] #880 - External subtree from file not found when loaded via inline XML
141+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/880
142+
- **Summary:** When a subtree is registered via `registerBehaviorTreeFromFile()` and then referenced from inline XML via `createTreeFromText()`, the factory throws `"Can't find a tree with name: MyTree"`.
143+
- **Component:** `src/xml_parsing.cpp`, `src/bt_factory.cpp`
144+
- **Test file:** `tests/gtest_factory.cpp`
145+
146+
### [ ] #672 - Stack buffer overflow in xml_parsing.cpp
147+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/672
148+
- **Summary:** AddressSanitizer detects a stack-buffer-overflow in `xml_parsing.cpp` when parsing certain XML tree definitions. Memory safety issue in the XML parser.
149+
- **Component:** `src/xml_parsing.cpp`
150+
- **Test file:** `tests/gtest_factory.cpp` (or new)
151+
152+
---
153+
154+
## BehaviorTreeFactory
155+
156+
### [ ] #1046 - Heap use-after-free when factory is destroyed before ticking
157+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/1046
158+
- **Summary:** If `BehaviorTreeFactory` is destroyed before its created tree is ticked, `NodeConfig` holds a dangling pointer to `TreeNodeManifest` owned by the factory. Accessing it causes heap-use-after-free.
159+
- **Component:** `src/bt_factory.cpp`, `include/behaviortree_cpp/tree_node.h`
160+
- **Test file:** `tests/gtest_factory.cpp`
161+
162+
### [ ] #937 - BehaviorTreeFactory cannot be returned by value
163+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/937
164+
- **Summary:** Making `BehaviorTreeFactory` non-copyable also inadvertently prevents returning it by value from functions. Some compilers do not apply guaranteed copy elision in all contexts.
165+
- **Component:** `include/behaviortree_cpp/bt_factory.h`
166+
- **Test file:** `tests/gtest_factory.cpp` (or new)
167+
168+
### [ ] #934 - Segfault when substituting subtree
169+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/934
170+
- **Summary:** `factory.loadSubstitutionRuleFromJSON()` followed by `factory.createTree()` causes a segmentation fault when the substitution target is a subtree node.
171+
- **Component:** `src/bt_factory.cpp`
172+
- **Test file:** `tests/gtest_factory.cpp`
173+
174+
### [ ] #930 - Mock substitution with registerSimpleAction causes tree to hang
175+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/930
176+
- **Summary:** When using mock node substitution rules with `registerSimpleAction()`, the behavior tree gets stuck after an async delay finishes. The substituted node does not behave like the original.
177+
- **Component:** `src/bt_factory.cpp`
178+
- **Test file:** `tests/gtest_factory.cpp` (or new)
179+
180+
---
181+
182+
## Loggers
183+
184+
### [ ] #1057 - Groot2Publisher enters infinite loop on exception
185+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/1057
186+
- **Summary:** `Groot2Publisher` can enter an infinite loop during destruction. When `active_server` is set to `false`, it can be reset to `true` depending on timing, causing `server_thread.join()` to hang indefinitely.
187+
- **Component:** `src/loggers/groot2_publisher.cpp`
188+
- **Test file:** New test needed
189+
190+
---
191+
192+
## Tree Execution
193+
194+
### [ ] #686 - haltTree doesn't effectively halt tickWhileRunning
195+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/686
196+
- **Summary:** `haltTree()` resets tree status to `IDLE`, which only breaks the inner loop of `tickRoot`. The outer loop then restarts execution because `IDLE` satisfies its condition, so the tree is never truly halted with `tickWhileRunning()`.
197+
- **Component:** `src/bt_factory.cpp`, `include/behaviortree_cpp/bt_factory.h`
198+
- **Test file:** `tests/gtest_tree.cpp` (or new)
199+
200+
---
201+
202+
## TreeNode
203+
204+
### [ ] #861 - Tick time monitoring can measure 0 due to instruction reordering
205+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/861
206+
- **Summary:** The `std::chrono` calls around `executeTick()` in `tree_node.cpp` can be reordered by the compiler, resulting in a measured tick duration of 0.
207+
- **Component:** `src/tree_node.cpp`
208+
- **Test file:** `tests/gtest_tree.cpp` (or new)
209+
210+
---
211+
212+
## JSON Exporter
213+
214+
### [ ] #989 - JsonExporter throws during vector conversion
215+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/989
216+
- **Summary:** A `bad_function_call` exception is thrown when converting `std::vector` types to JSON via `JsonExporter`. The conversion function is not properly registered for vector types.
217+
- **Component:** `include/behaviortree_cpp/json_export.h`
218+
- **Test file:** New test needed
219+
220+
---
221+
222+
## Build / Dependencies
223+
224+
### [ ] #959 - Version conflict with embedded nlohmann::json causes UB
225+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/959
226+
- **Summary:** The library embeds `nlohmann::json` v3.11.3 in its headers. If the user's codebase uses a different version, ODR violations occur depending on header include order, potentially causing undefined behavior.
227+
- **Component:** `3rdparty/`, `CMakeLists.txt`
228+
- **Test file:** Build system test / CMake configuration
229+
230+
---
231+
232+
## Platform-Specific
233+
234+
### [ ] #869 - Read access violation on Windows debug build
235+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/869
236+
- **Summary:** Running on VS 2022 in Debug mode causes a read access violation in string hashing within the library DLL. Release mode works fine, suggesting a debug-mode-specific memory issue.
237+
- **Component:** Core library (platform-specific)
238+
- **Test file:** Requires Windows environment
239+
240+
---
241+
242+
## Summary
243+
244+
| Category | Count | Issue Numbers |
245+
|----------|-------|---------------|
246+
| Scripting / Script Parser | 3 | #1029, #923, #832 |
247+
| Ports / Type Conversion | 5 | #1065, #982, #969, #948, #858 |
248+
| Blackboard | 3 | #974, #942, #408 |
249+
| Control Nodes (Parallel) | 3 | #1045, #1041, #966 |
250+
| Control Nodes (Reactive) | 2 | #1031, #917 |
251+
| XML Parsing | 4 | #979, #883, #880, #672 |
252+
| BehaviorTreeFactory | 4 | #1046, #937, #934, #930 |
253+
| Loggers | 1 | #1057 |
254+
| Tree Execution | 1 | #686 |
255+
| TreeNode | 1 | #861 |
256+
| JSON Exporter | 1 | #989 |
257+
| Build / Dependencies | 1 | #959 |
258+
| Platform-Specific | 1 | #869 |
259+
| **Total** | **30** | |

‎include/behaviortree_cpp/decorators/loop_node.h‎

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include "behaviortree_cpp/decorator_node.h"
1616

1717
#include <deque>
18+
#include <vector>
1819

1920
namespace BT
2021
{
@@ -75,7 +76,37 @@ class LoopNode : public DecoratorNode
7576
static_queue_ ? AnyPtrLocked() : getLockedPortContent("queue");
7677
if(any_ref)
7778
{
78-
current_queue_ = any_ref.get()->cast<SharedQueue<T>>();
79+
// Try SharedQueue<T> first, then fall back to std::vector<T>.
80+
// This allows upstream nodes that output vector<T> to be used
81+
// directly with LoopNode without manual conversion. Issue #969.
82+
auto queue_result = any_ref.get()->tryCast<SharedQueue<T>>();
83+
if(queue_result)
84+
{
85+
current_queue_ = queue_result.value();
86+
}
87+
else if(!current_queue_)
88+
{
89+
// Only convert on first read; after that, use the
90+
// already-converted current_queue_ (which gets popped).
91+
auto vec_result = any_ref.get()->tryCast<std::vector<T>>();
92+
if(vec_result)
93+
{
94+
// Accept std::vector<T> from upstream nodes. Issue #969.
95+
const auto& vec = vec_result.value();
96+
current_queue_ = std::make_shared<std::deque<T>>(vec.begin(), vec.end());
97+
}
98+
else if(any_ref.get()->isString())
99+
{
100+
// Accept string values (e.g. from subtree remapping). Issue #1065.
101+
auto str = any_ref.get()->cast<std::string>();
102+
current_queue_ = convertFromString<SharedQueue<T>>(str);
103+
}
104+
else
105+
{
106+
throw RuntimeError("LoopNode: port 'queue' must contain either "
107+
"SharedQueue<T>, std::vector<T>, or a string");
108+
}
109+
}
79110
}
80111

81112
if(current_queue_ && !current_queue_->empty())
@@ -114,8 +145,9 @@ class LoopNode : public DecoratorNode
114145

115146
static PortsList providedPorts()
116147
{
117-
// we mark "queue" as BidirectionalPort, because the original element is modified
118-
return { BidirectionalPort<SharedQueue<T>>("queue"),
148+
// Use an untyped BidirectionalPort to accept both SharedQueue<T>
149+
// and std::vector<T> without triggering a port type mismatch. Issue #969.
150+
return { BidirectionalPort("queue"),
119151
InputPort<NodeStatus>("if_empty", NodeStatus::SUCCESS,
120152
"Status to return if queue is empty: "
121153
"SUCCESS, FAILURE, SKIPPED"),

‎tests/gtest_ports.cpp‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -704,3 +704,58 @@ TEST(PortTest, DefaultEmptyVector_Issue982)
704704
<< " element(s). First element: \""
705705
<< (result.empty() ? "" : result[0]) << "\"";
706706
}
707+
708+
// Issue #969: LoopNode<T> uses SharedQueue<T> (shared_ptr<deque<T>>) for its queue
709+
// port, but upstream nodes often produce std::vector<T>. This type mismatch causes
710+
// tree creation to fail.
711+
class ProduceVectorDoubleAction : public SyncActionNode
712+
{
713+
public:
714+
ProduceVectorDoubleAction(const std::string& name, const NodeConfig& config)
715+
: SyncActionNode(name, config)
716+
{}
717+
718+
NodeStatus tick() override
719+
{
720+
std::vector<double> vec = { 10.0, 20.0, 30.0 };
721+
setOutput("numbers", vec);
722+
return NodeStatus::SUCCESS;
723+
}
724+
725+
static PortsList providedPorts()
726+
{
727+
return { BT::OutputPort<std::vector<double>>("numbers") };
728+
}
729+
};
730+
731+
TEST(PortTest, LoopNodeAcceptsVector_Issue969)
732+
{
733+
// An upstream node outputs std::vector<double>, and LoopDouble should be
734+
// able to iterate over it without requiring manual conversion to SharedQueue.
735+
std::string xml_txt = R"(
736+
<root BTCPP_format="4">
737+
<BehaviorTree ID="MainTree">
738+
<Sequence>
739+
<ProduceVectorDouble numbers="{nums}" />
740+
<LoopDouble queue="{nums}" value="{val}">
741+
<CollectDouble value="{val}" />
742+
</LoopDouble>
743+
</Sequence>
744+
</BehaviorTree>
745+
</root>
746+
)";
747+
748+
std::vector<double> collected;
749+
750+
BehaviorTreeFactory factory;
751+
factory.registerNodeType<ProduceVectorDoubleAction>("ProduceVectorDouble");
752+
factory.registerNodeType<CollectDoubleAction>("CollectDouble", &collected);
753+
auto tree = factory.createTreeFromText(xml_txt);
754+
auto status = tree.tickWhileRunning();
755+
756+
ASSERT_EQ(status, NodeStatus::SUCCESS);
757+
ASSERT_EQ(collected.size(), 3u);
758+
EXPECT_DOUBLE_EQ(collected[0], 10.0);
759+
EXPECT_DOUBLE_EQ(collected[1], 20.0);
760+
EXPECT_DOUBLE_EQ(collected[2], 30.0);
761+
}

0 commit comments

Comments
 (0)