Skip to content

Prevent executor race condition#421

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

Prevent executor race condition#421
mjcarroll wants to merge 1 commit into
rollingfrom
mjcarroll/executor_race_condition_hypothesis

Conversation

@mjcarroll
Copy link
Copy Markdown
Contributor

@mjcarroll mjcarroll commented May 30, 2026

Description

This PR fixes a sporadic hang/deadlock in the camera_info_manager unit tests (https://ci.ros2.org/view/nightly/job/nightly_linux-aarch64_repeated/3582/testReport/junit/(root)/projectroot/camera_info_manager_unit_test/)

Root Cause

In tests/unit_test.cpp, the CameraInfoManagerTesting fixture spins an executor in a background thread during SetUp():

executor_thread = std::thread([this]() {executor->spin();});

And cancels/joins it during TearDown() :

executor->cancel();
executor_thread.join();

For very fast test cases, purely synchronous test cases the main thread completes and calls TearDown() almost immediately after SetUp(). This creates a race condition in rclcpp::Executor:

  1. TearDown() calls executor->cancel() , setting the executor's internal spinning atomic flag to false .
  2. The background thread subsequently starts executing spin() , which performs spinning.exchange(true) . This unconditionally
    sets spinning back to true .
  3. The executor enters the spin loop and blocks on wait_for_work() indefinitely, as cancel() has already been called. The
    thread never joins, causing the test suite to hang.

Is this user-facing behavior change?

No (test code only).

Did you use Generative AI?

Yes, this pull request description and fix were prepared with the assistance of Gemini CLI.

Assisted-by: Gemini CLI:Gemini 3.5 Flash [run_command, view_file, grep_search, replace_file_content]

Additional Information

I think this is actually a valid race condition that we are going to potentially need to address in the executor?

This is a hypothesis that works locally, but I would like to run through
CI.

Signed-off-by: Michael Carroll <mjcarroll.oss@gmail.com>
@mjcarroll mjcarroll self-assigned this May 30, 2026
@mjcarroll
Copy link
Copy Markdown
Contributor Author

Pulls: #421
Gist: https://gist.githubusercontent.com/mjcarroll/531ec5be0745898904012fcabe99cca9/raw/052db77e388b9bd6c918eb2f3d36e7fd91782ba3/ros2.repos
BUILD args: --packages-up-to camera_info_manager
TEST args: --packages-select camera_info_manager --ctest-args --repeat until-fail:200
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/19409

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant