Skip to content

Data race on ResourceManager::handle_exceptions during initialization #3613

Description

@WaWOscarQAQ

Describe the bug

The StateInterface introspection callback reads the handle value without handle_mutex_. Concurrent calls to Handle::set_value() write the same value under this mutex, causing a data race.

To Reproduce

Requires ROS 2 Lyrical, colcon, rosdep, Clang 18.

Build with TSan:

source /opt/ros/lyrical/setup.bash
mkdir -p ~/ros2_control_tsan_ws/src
cd ~/ros2_control_tsan_ws
git clone https://github.com/ros-controls/ros2_control.git src/ros2_control
git -C src/ros2_control checkout ffcf444a69a5bdae7b19d09fc7e45e0224386231
rosdep install --from-paths src --ignore-src -r -y
colcon build --packages-up-to controller_manager --cmake-args \
  -DBUILD_TESTING=OFF \
  -DCMAKE_CXX_COMPILER=clang++-18 \
  '-DCMAKE_CXX_FLAGS=-fsanitize=thread -g -O1 -fno-omit-frame-pointer --gcc-install-dir=/usr/lib/gcc/x86_64-linux-gnu/15' \
  -DCMAKE_SHARED_LINKER_FLAGS=-fsanitize=thread \
  -DCMAKE_EXE_LINKER_FLAGS=-fsanitize=thread

Terminal 1:

cd ~/ros2_control_tsan_ws
source install/setup.bash
export ROS_DOMAIN_ID=181
export TSAN_OPTIONS='halt_on_error=0:external_symbolizer_path=/usr/bin/llvm-symbolizer-18'
ros2 run controller_manager ros2_control_node --ros-args \
  -p update_rate:=1000 \
  -p diagnostic_updater.period:=0.001 \
  -p overruns.print_warnings:=false \
  -p 'hardware_components_initial_state.unconfigured:=[TestSystem]'

Terminal 2, once the node is waiting for the robot description:

cd ~/ros2_control_tsan_ws
source install/setup.bash
export ROS_DOMAIN_ID=181
robot_description='<robot name="tsan_repro">
  <link name="base"/><link name="tip"/>
  <joint name="joint1" type="revolute">
    <parent link="base"/><child link="tip"/>
    <limit lower="-1" upper="1" effort="1" velocity="1"/>
  </joint>
  <ros2_control name="TestSystem" type="system">
    <hardware><plugin>mock_components/GenericSystem</plugin></hardware>
    <joint name="joint1">
      <command_interface name="position"/><state_interface name="position"/>
    </joint>
  </ros2_control>
</robot>'
ros2 topic pub --once --qos-durability transient_local \
  /robot_description std_msgs/msg/String "{data: '$robot_description'}"

After hardware initialization, configure and unconfigure it repeatedly in Terminal 2:

for i in {1..20}; do
  ros2 service call --timeout 5 \
    /controller_manager/set_hardware_component_state \
    controller_manager_msgs/srv/SetHardwareComponentState \
    "{name: TestSystem, target_state: {id: 2, label: inactive}}"
  ros2 service call --timeout 5 \
    /controller_manager/set_hardware_component_state \
    controller_manager_msgs/srv/SetHardwareComponentState \
    "{name: TestSystem, target_state: {id: 1, label: unconfigured}}"
done

Watch Terminal 1 for the TSan report below; repeat the loop if needed. The race was reproduced with an equivalent rclpy service client. The CLI service loop above was not fully validated because the local TSan overlay caused a Python launcher error.

Expected behavior

Introspection sampling should be synchronized with Handle::set_value().

Environment

  • Ubuntu 26.04.1 LTS, x86_64; ROS 2 Lyrical.
  • ros2_control 6.9.0, commit ffcf444a69a5bdae7b19d09fc7e45e0224386231.
  • Clang 18 ThreadSanitizer.

Additional context

TSan output from the unmodified revision:

Write of size 8:
  Handle::set_value<double>()
    hardware_interface/include/hardware_interface/handle.hpp:487
  GenericSystem::on_configure()
    hardware_interface/src/mock_components/generic_system.cpp:321

Previous read of size 8:
  StateInterface::registerIntrospection()::{lambda}::operator()()
    hardware_interface/include/hardware_interface/handle.hpp:666
  pal_statistics::RegistrationList::doUpdate()
  ros2_control_node.cpp:154

Proposed fix

In hardware_interface/include/hardware_interface/handle.hpp, add this guard at the start of the sampling lambda in StateInterface::registerIntrospection():

std::shared_lock<std::shared_mutex> lock(handle_mutex_, std::try_to_lock);
if (!lock.owns_lock())
{
  return std::numeric_limits<double>::quiet_NaN();
}

Keep the existing reads and value conversion under this lock. This synchronizes sampling with Handle::set_value() without blocking when the mutex is unavailable.

Review the same unlocked sampling in CommandInterface::registerIntrospection() for the same fix.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions