Thread-safe issue on GuardCondition

[evshary]

Operating System:

Ubuntu 22.04

ROS version or commit hash:

humble

Steps to reproduce issue

1. Run the Autoware with rmw_zenoh on https://autowarefoundation.github.io/autoware-documentation/main/tutorials/ad-hoc-simulation/planning-simulation/
2. After giving an initial pose, we can set the goal successfully once
3. However, it's stuck if we set it twice.

More detail log is here

Expected behavior

Able to plan the route several times

Actual behavior

Failed to plan the route

Additional information

After some investigation, we found that the issue is related to thread-safe issue in GuardCondition.
If we make the composable node in Autoware running with only 1 thread, it works fine.
If we don't release the guard_condition->data, it also works fine.

rmw_zenoh/rmw_zenoh_cpp/src/rmw_zenoh.cpp

Line 1994 in 2c91032

allocator->deallocate(guard_condition->data, allocator->state);

I think it's use-after-free error. When guard_condition->data is still using inside rmw_wait, another thread calls rmw_destroy_guard_condition and caused the problem.
I've tried both ways to solve the issues:

1. Wait until rmw_wait exits to destroy the GuardCondition
   humble...evshary:rmw_zenoh:wait_fix
2. Record all the addresses and ignore them while using the invalid one in rmw_wait
   humble...evshary:rmw_zenoh:ignore_invalid_address

Both approaches can solve the issue, but the second might run into trouble sometimes. (about once every ten times)

[component_container_mt-34] terminate called after throwing an instance of 'rclcpp::exceptions::RCLError'                                                                                                   
[component_container_mt-34]   what():  failed to add guard condition to wait set: guard condition implementation is invalid, at ./src/rcl/guard_condition.c:172, at ./src/rcl/wait.c:460

I suppose that guard condition might be used even after rmw_wait. I would like to know more about your opinion on how to deal with this issue.

[Yadunund]

@evshary do you know if the issue is reproducible on rolling? (It seems like it would be but just want to make sure)

[evshary]

@Yadunund I can't confirm that because Autoware only supports Humble currently. I'm trying to reproduce it with a minimal reproducible example, but haven't managed to do that.
Do you know if it's allowed in the original design that rmw_wait uses the guard condition after rmw_destroy_guard_condition on a different thread?

[evshary]

Okay, here is the minimal reproducible example
https://github.com/evshary/ros2-simple-examples/blob/e2a7ae145bffbbe71654512bc51db4e48c4a8182/simple_examples_cpp/src/timer/timer.cpp

I did the comparison between humble and rolling:

• Humble: happened in a short period of time
• Rolling: need to run it for a long time. (I found the issue an hour later...)

Perhaps this is related to the refactor on waitset: ros2/rclcpp#2142

If I applied the patch I mentioned before, it still ran into panic, but it happened less frequently.

[component_container_mt-34] terminate called after throwing an instance of 'rclcpp::exceptions::RCLError'                                                                                                   
[component_container_mt-34]   what():  failed to add guard condition to wait set: guard condition implementation is invalid, at ./src/rcl/guard_condition.c:172, at ./src/rcl/wait.c:460

I'm not pretty sure if we should fix the issue in rmw_zenoh or rclcpp. It might depend on whether we allow rmw_wait use the guard condition after rmw_destroy_guard_condition on a different thread.

[YuanYuYuan]

I attempted to reproduce the issue using ROS 2 Humble with different RMW implementations. The observed behavior is as follows:

rmw_zenoh_cpp

• Deadlock

rmw_cyclonedds_cpp

• Sometimes crashed with this error

terminate called after throwing an instance of 'rclcpp::exceptions::RCLError'
  what():  failed to add guard condition to wait set: guard condition implementation is invalid, at /build/rcl-release-release-humble-rcl-5.3.9-1/src/rcl/guard_condition.c:172, at /build/rcl-release-release-humble-rcl-5.3.9-1/src/rcl/wait.c:460
[ros2run]: Aborted

rmw_fastrtps_cpp

• Deadlock
• Crashed with same RCLError as above

terminate called after throwing an instance of 'rclcpp::exceptions::RCLError'
  what():  failed to add guard condition to wait set: guard condition implementation is invalid, at /build/rcl-release-release-humble-rcl-5.3.9-1/src/rcl/guard_condition.c:172, at /build/rcl-release-release-humble-rcl-5.3.9-1/src/rcl/wait.c:460
[ros2run]: Aborted

• Segfault

Thread 1 "timer" received signal SIGSEGV, Segmentation fault.
0x00007ffff70df59f in rmw_fastrtps_shared_cpp::__rmw_wait(char const*, rmw_subscriptions_s*, rmw_guard_conditions_s*, rmw_services_s*, rmw_clients_s*, rmw_events_s*, rmw_wait_set_s*, rmw_time_s const*) () from /nix/store/ipqymfdxzp8z72sbk4j616rw15csqkw7-ros-env/lib/librmw_fastrtps_shared_cpp.so
(gdb) bt
#0  0x00007ffff70df59f in rmw_fastrtps_shared_cpp::__rmw_wait(char const*, rmw_subscriptions_s*, rmw_guard_conditions_s*, rmw_services_s*, rmw_clients_s*, rmw_events_s*, rmw_wait_set_s*, rmw_time_s const*) () from /nix/store/ipqymfdxzp8z72sbk4j616rw15csqkw7-ros-env/lib/librmw_fastrtps_shared_cpp.so
#1  0x00007ffff713e3ab in rmw_wait () from /nix/store/ipqymfdxzp8z72sbk4j616rw15csqkw7-ros-env/lib/librmw_fastrtps_cpp.so
#2  0x00007ffff7fa3440 in rcl_wait () from /nix/store/ipqymfdxzp8z72sbk4j616rw15csqkw7-ros-env/lib/librcl.so
#3  0x00007ffff7d530fb in rclcpp::Executor::wait_for_work(std::chrono::duration<long, std::ratio<1l, 1000000000l> >) ()
   from /nix/store/ipqymfdxzp8z72sbk4j616rw15csqkw7-ros-env/lib/librclcpp.so
#4  0x00007ffff7d53633 in rclcpp::Executor::get_next_executable(rclcpp::AnyExecutable&, std::chrono::duration<long, std::ratio<1l, 1000000000l> >) ()
   from /nix/store/ipqymfdxzp8z72sbk4j616rw15csqkw7-ros-env/lib/librclcpp.so
#5  0x00007ffff7d5789d in rclcpp::executors::MultiThreadedExecutor::run(unsigned long) () from /nix/store/ipqymfdxzp8z72sbk4j616rw15csqkw7-ros-env/lib/librclcpp.so
#6  0x00007ffff7d57d55 in rclcpp::executors::MultiThreadedExecutor::spin() () from /nix/store/ipqymfdxzp8z72sbk4j616rw15csqkw7-ros-env/lib/librclcpp.so
#7  0x000000000040461b in main (argc=<optimized out>, argv=<optimized out>)
    at /ws-humble/src/ros2-simple-examples/simple_examples_cpp/src/timer/timer.cpp:73

[evshary]

This should be the issue in rclcpp, and it's already fixed in the latest rolling. Let's close the issue.

[Yadunund]

@evshary thanks for investigating!