Skip to content

Fix MultiThreadedExecutor busy wait on busy callback groups - #3395

Draft
mjcarroll wants to merge 1 commit into
rollingfrom
mjcarroll/fix_mte_busy_wait
Draft

mjcarroll wants to merge 1 commit into
rollingfrom
mjcarroll/fix_mte_busy_wait

Conversation

@mjcarroll

@mjcarroll mjcarroll commented Oct 30, 2026 •

Copy link
Copy Markdown
Member

Description

Fixes the remaining flake in TestBusyWaiting seen in nightly_linux_repeated:

test_executors_busy_waiting.cpp:109
Expected: (waitable->get_is_ready_call_count()) < (10u * this->waitable->get_count()), actual: 2507 vs 20

This is the same symptom as #3154. #3155 fixed a race in the test, but this one is a real busy wait in the MultiThreadedExecutor.

Root cause

When thread A takes an entity from a MutuallyExclusive callback group, can_be_taken_from becomes false. If the entity (or another one in the group) becomes ready again while A is still executing, the idle thread B:

  1. wakes from wait_for_work(),
  2. skips the entity in get_next_ready_executable() because its group is busy,
  3. calls wait_for_work() again with the same collection, since nothing set entities_need_rebuild_.

For level-triggered entities (a subscription with queued data, or TestWaitable, which re-triggers its guard condition in add_to_wait_set() while trigger_count_ > 0), the wait returns immediately and B spins until A finishes and the interrupt_guard_condition_ triggers a rebuild.

build_entities_collection() already excludes groups that can't be taken from, but that only helps if a rebuild happens while the group is busy. In test_spin that depends on whether the notify waitable fired in the same wait that picked up the first execution, which is a race. The busy wait then lasts as long as the first callback, which waits on the main thread and is long on a loaded CI machine.

Fix

In Executor::get_next_ready_executable(), set entities_need_rebuild_ whenever a ready entity is skipped because its callback group is gone or can't be taken from. The next wait then excludes the busy group, and the existing post-execution interrupt_guard_condition_ trigger in MultiThreadedExecutor::run() adds it back once it is released. The extra rebuild only happens in the case that would otherwise busy wait

If an entity becomes ready while its mutually exclusive callback group is
executing in another thread, the idle executor threads skipped it in
get_next_ready_executable() but kept it in their wait set. Level-triggered
entities (e.g. a subscription with queued data, or a waitable that
re-triggers its guard condition) then woke the wait set immediately, so the
idle threads busy waited until the callback group was released.

build_entities_collection() already excludes callback groups that cannot be
taken from, but nothing requested a rebuild when this happened. Request a
rebuild whenever a ready entity is skipped because of its callback group;
the group is added back by the rebuild that follows the interrupt guard
condition triggered once the group is released.

This was the cause of the flaky TestBusyWaiting/MultiThreadedExecutor.test_spin
failures in nightly_linux_repeated. Adds a deterministic regression test.

Signed-off-by: Michael Carroll <mjcarroll.oss@gmail.com>
@github-actions

Copy link
Copy Markdown

ABI Compliance Check

✅ Verdict: compatible

Library Verdict Summary
libcomponent_manager.so ✅ compatible No ABI changes detected.
librclcpp.so ✅ compatible No ABI changes detected.
librclcpp_action.so ✅ compatible No ABI changes detected.
librclcpp_lifecycle.so ✅ compatible No ABI changes detected.
✅ libcomponent_manager.so — full abidiff report

Compared:

  • Base: lib-base/libcomponent_manager.so
  • Head: lib-pr/libcomponent_manager.so @ 08558e7
(empty report — no differences printed by abidiff)
✅ librclcpp.so — full abidiff report

Compared:

  • Base: lib-base/librclcpp.so
  • Head: lib-pr/librclcpp.so @ 08558e7
(empty report — no differences printed by abidiff)
✅ librclcpp_action.so — full abidiff report

Compared:

  • Base: lib-base/librclcpp_action.so
  • Head: lib-pr/librclcpp_action.so @ 08558e7
(empty report — no differences printed by abidiff)
✅ librclcpp_lifecycle.so — full abidiff report

Compared:

  • Base: lib-base/librclcpp_lifecycle.so
  • Head: lib-pr/librclcpp_lifecycle.so @ 08558e7
(empty report — no differences printed by abidiff)

Updated for commit 08558e7 · check run · suppressions: /home/runner/work/_temp/ros2-abi-suppressions.txt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant