Bounds-check EventHandlerBase::is_ready() against the wait set (#2376)

is_ready() indexed wait_set.events with wait_set_event_index_, which is only
assigned once the handler is added to that wait set via add_to_wait_set(). If
is_ready() runs first (e.g. a publisher/subscription event thread racing node
setup), the index is uninitialized or refers to a different wait set, so the
access reads out of bounds and can segfault (observed with ASan during lifecycle
configure).

Initialize wait_set_event_index_ and make is_ready() return false when the index
is not within the wait set's events instead of dereferencing it. Initializing
alone is not sufficient (index 0 may still not be this handler's slot), so the
bounds check is the actual guard.

Adds a regression test that calls is_ready() on a handler that was never added to
a wait set.

Refs #2376

Co-authored-by: Mike Wake <michael.wake@aosgrp.com.au>
Generated-by: Claude Opus 4.8 (Anthropic)
Signed-off-by: Pavel Guzenfeld <pavelguzenfeld@gmail.com>
Signed-off-by: Mike Wake <michael.wake@aosgrp.com.au>

Author: PavelGuzenfeld

rclcpp/include/rclcpp/event_handler.hpp

@@ -17,6 +17,7 @@
 
 #include <atomic>
 #include <functional>
+#include <limits>
 #include <memory>
 #include <mutex>
 #include <stdexcept>
@@ -247,7 +248,9 @@ class EventHandlerBase : public Waitable
   std::function<void(size_t)> on_new_event_callback_{nullptr};
 
   rcl_event_t event_handle_;
-  size_t wait_set_event_index_;
+  // Sentinel that is always out of range, so is_ready() reports "not in any wait set"
+  // until add_to_wait_set() assigns the real index. See ros2/rclcpp#2376.
+  size_t wait_set_event_index_ = std::numeric_limits<size_t>::max();
 };
 
 template<typename EventCallbackT, typename ParentHandleT>

rclcpp/src/rclcpp/event_handler.cpp

@@ -69,6 +69,13 @@ EventHandlerBase::add_to_wait_set(rcl_wait_set_t & wait_set)
 bool
 EventHandlerBase::is_ready(const rcl_wait_set_t & wait_set)
 {
+  // wait_set_event_index_ is only valid once this handler has been added to this wait
+  // set via add_to_wait_set(). If is_ready() is reached before that (e.g. an event
+  // thread racing node setup, see ros2/rclcpp#2376), the index may not refer to a slot
+  // in this wait set, so guard against an out-of-bounds read.
+  if (wait_set_event_index_ >= wait_set.size_of_events) {
+    return false;
+  }
   return wait_set.events[wait_set_event_index_] == &event_handle_;
 }
 

rclcpp/test/rclcpp/test_qos_event.cpp

@@ -338,6 +338,28 @@ TEST_F(TestQosEvent, add_to_wait_set) {
   }
 }
 
+// Regression for ros2/rclcpp#2376: is_ready() must not index wait_set.events with an
+// uninitialized/stale wait_set_event_index_ when the handler is not in the wait set
+// (e.g. an event thread racing node setup). It must return false, not read out of bounds.
+TEST_F(TestQosEvent, is_ready_when_not_in_wait_set) {
+  auto publisher = node->create_publisher<test_msgs::msg::Empty>(topic_name, 10);
+  auto rcl_handle = publisher->get_publisher_handle();
+
+  auto callback = [](int) {};
+
+  const rcl_publisher_event_type_t event_type =
+    !rclcpp::PublisherBase::event_type_is_supported(RCL_PUBLISHER_OFFERED_DEADLINE_MISSED) ?
+    RCL_PUBLISHER_MATCHED : RCL_PUBLISHER_OFFERED_DEADLINE_MISSED;
+
+  rclcpp::EventHandler<decltype(callback), decltype(rcl_handle)> handler(
+    callback, rcl_publisher_event_init, rcl_handle, event_type);
+
+  // The handler has not been added to any wait set, so its event index is not valid for
+  // this (empty) wait set; is_ready() must report not-ready instead of dereferencing it.
+  rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set();
+  EXPECT_FALSE(handler.is_ready(wait_set));
+}
+
 TEST_F(TestQosEvent, test_on_new_event_callback)
 {
   if (!rclcpp::SubscriptionBase::event_type_is_supported(