WaitSet failed remove clears entity ownership and permits the same guard condition in two wait sets

[Yuhx141]

Required information

• Operating system: Ubuntu 24.04.4 LTS
• Installation: ROS 2 Jazzy binaries
• rclcpp: 28.1.21
• Jazzy source commit: 2209942eb1361fdaf48ec8512b6dccf70235bccb
• The affected path is also present in rolling commit eee4b508d4357c81c0847f922617e5f498d1d4b7
• RMW: rmw_fastrtps_cpp (the reproducer does not create DDS entities)

Steps to reproduce

This is a complete standalone program and build file:

source /opt/ros/jazzy/setup.bash
WS=/tmp/rclcpp-waitset-remove
mkdir -p "$WS"
cd "$WS"

cat > CMakeLists.txt <<'CMAKE'
cmake_minimum_required(VERSION 3.16)
project(rclcpp_waitset_probe LANGUAGES CXX)
find_package(ament_cmake REQUIRED)
find_package(rclcpp REQUIRED)
add_executable(rclcpp_waitset_probe rclcpp_waitset_probe.cpp)
target_compile_features(rclcpp_waitset_probe PRIVATE cxx_std_17)
ament_target_dependencies(rclcpp_waitset_probe rclcpp)
CMAKE

cat > rclcpp_waitset_probe.cpp <<'CPP'
#include <iostream>
#include <memory>
#include <stdexcept>
#include "rclcpp/rclcpp.hpp"

int main(int argc, char ** argv)
{
  rclcpp::init(argc, argv);
  bool direct_cross_add_threw = false;
  bool wrong_remove_threw = false;
  bool post_failure_cross_add_threw = false;
  {
    rclcpp::WaitSet owner;
    rclcpp::WaitSet other;
    auto gc = std::make_shared<rclcpp::GuardCondition>();
    owner.add_guard_condition(gc);
    try { other.add_guard_condition(gc); }
    catch (const std::runtime_error &) { direct_cross_add_threw = true; }
    try { other.remove_guard_condition(gc); }
    catch (const std::runtime_error &) { wrong_remove_threw = true; }
    try { other.add_guard_condition(gc); }
    catch (const std::runtime_error &) { post_failure_cross_add_threw = true; }
  }
  rclcpp::shutdown();
  std::cout
    << "direct_cross_add_threw=" << direct_cross_add_threw << "\n"
    << "wrong_remove_threw=" << wrong_remove_threw << "\n"
    << "post_failure_cross_add_threw=" << post_failure_cross_add_threw << "\n";
  return direct_cross_add_threw && wrong_remove_threw && post_failure_cross_add_threw ? 0 : 1;
}
CPP

cmake -S . -B build
cmake --build build
./build/rclcpp_waitset_probe

Expected behavior

The failed removal from other must leave ownership unchanged, so the final add to other should throw:

direct_cross_add_threw=1
wrong_remove_threw=1
post_failure_cross_add_threw=1

Actual behavior

The final add succeeds and the program exits with status 1:

direct_cross_add_threw=1
wrong_remove_threw=1
post_failure_cross_add_threw=0

This reproduced 3/3 with the installed Jazzy release, 3/3 with the current Jazzy header, and 3/3 with the rolling implementation.

Additional information

remove_guard_condition clears in_use_by_wait_set before dynamic storage verifies that the guard condition belongs to this wait set. Storage then throws, leaving the flag clear while the original wait set still contains the entity.

Moving storage validation/removal before clearing the flag makes the probe pass. Timer, client, service, waitable, and subscription removal paths use similar ordering, so the failure-atomicity behavior may need review beyond this guard-condition trigger.