DecoratorNode: don't discard the result of an asynchronous child (root cause of flaky RepeatTestAsync)

[facontidavide]

Bug

RepeatTestAsync.RepeatTestAsync is not just a slow-runner flake: it exposes a real race in DecoratorNode::executeTick():

const NodeStatus status = TreeNode::executeTick();   // decorator's tick() sees the child RUNNING
const NodeStatus child_status = child()->status();   // ...the action thread sets SUCCESS in between
if(child_status == SUCCESS || child_status == FAILURE)
  child()->resetStatus();                            // result thrown away

With a ThreadedAction child, the thread can complete after the decorator's tick() read RUNNING and before this re-read. The child goes RUNNING -> SUCCESS -> IDLE without the decorator ever seeing SUCCESS, so on the next tick the action is started again: its side effects run twice. It affects any decorator with a ThreadedAction child, not only Repeat.

Trace of a failing iteration (0 IDLE, 1 RUNNING, 2 SUCCESS):

repeat: prev=0 child=1 count=1     <- action started
thread: tick=2                     <- action thread returns SUCCESS
repeat: prev=1 child=1 count=1     <- decorator still reads RUNNING; SUCCESS lands right after, and is reset
repeat: prev=0 child=1 count=1     <- child is IDLE again: started a second time, count not incremented

Reproduction on Linux under 4x CPU oversubscription: 246 failures / 12000 iterations (successCount 4 or 5 in the first phase, stray successes in the second). TSan reports no data race, confirming that the extra executions are sequential re-runs, not overlapping threads.

Fix

That reset is a safety net (from 2019) for decorators that do not reset a completed child. Keep it, but only when the decorator itself has completed; while the decorator is RUNNING, the child's status is left for the next tick() to consume. All the built-in decorators already call resetChild() explicitly on the paths where they keep running, and the full suite does not rely on the old behaviour.

Behaviour change to be aware of: a third-party decorator that returns RUNNING after its child completed and never calls resetChild() was implicitly covered by the old code; it now has to reset the child itself (as every built-in decorator does).

Tests

• New Decorator.ChildCompletedAfterTickIsNotDiscarded: reproduces the window deterministically (no timing), fails on master, passes here.
• Stress: 0 failures / 12000 iterations of RepeatTestAsync (plus the other decorator tests) under the same load.
• Full Debug suite 530/530, pre-commit/clang-tidy clean.

🤖 Generated with Claude Code

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud