From 240e393d352ffb37ce839d098d43826520bdf150 Mon Sep 17 00:00:00 2001
From: Tamaki Nishino <otamachan@gmail.com>
Date: Sat, 21 Mar 2026 17:57:21 +0900
Subject: [PATCH] Fix Event::set() re-entrant infinite loop

When a resumed waiter re-registers via clear() + wait() inside set()'s
while loop, waiters_ never becomes empty, causing an infinite loop.
Swap waiters_ into a local queue before iterating so new waiters added
during resume() are deferred to the next set() call.
---
 .../include/rclcpp_async/co_context.hpp       |  8 ++--
 rclcpp_async/test/test_event.cpp              | 46 +++++++++++++++++++
 2 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/rclcpp_async/include/rclcpp_async/co_context.hpp b/rclcpp_async/include/rclcpp_async/co_context.hpp
index 228c791..6f90f4c 100644
--- a/rclcpp_async/include/rclcpp_async/co_context.hpp
+++ b/rclcpp_async/include/rclcpp_async/co_context.hpp
@@ -516,9 +516,11 @@ inline void Event::WaitAwaiter::await_suspend(std::coroutine_handle<> h)
 inline void Event::set()
 {
   set_ = true;
-  while (!waiters_.empty()) {
-    auto w = waiters_.front();
-    waiters_.pop();
+  std::queue<Waiter> pending;
+  pending.swap(waiters_);
+  while (!pending.empty()) {
+    auto w = pending.front();
+    pending.pop();
     if (*w.active) {
       *w.active = false;
       ctx_.resume(w.handle);
diff --git a/rclcpp_async/test/test_event.cpp b/rclcpp_async/test/test_event.cpp
index c56facb..00177b9 100644
--- a/rclcpp_async/test/test_event.cpp
+++ b/rclcpp_async/test/test_event.cpp
@@ -187,6 +187,52 @@ TEST_F(EventTest, CancelDuringWait)
   EXPECT_TRUE(was_cancelled);
 }
 
+TEST_F(EventTest, SetDoesNotLoopWhenWaiterReregisters)
+{
+  // Reproduces the re-entrant infinite loop bug:
+  // 1. set() resumes a waiter synchronously
+  // 2. The waiter checks a condition, finds it unsatisfied, calls clear() + wait()
+  // 3. wait() re-registers a waiter in waiters_
+  // 4. set() sees waiters_ is non-empty and loops forever
+
+  Event event(*ctx_);
+  int value = 0;
+  int resume_count = 0;
+
+  auto consumer = [&]() -> Task<void> {
+    while (value != 42) {
+      event.clear();
+      co_await event.wait();
+      resume_count++;
+    }
+  };
+
+  auto consumer_task = ctx_->create_task(consumer());
+
+  // Spin to let consumer suspend on wait()
+  for (int i = 0; i < 10; i++) {
+    executor_.spin_some();
+    std::this_thread::sleep_for(1ms);
+  }
+  EXPECT_EQ(resume_count, 0);
+
+  // set() with wrong value — consumer will clear + re-wait.
+  // With the bug, this never returns (infinite loop).
+  value = 1;
+  event.set();
+
+  // If we get here, set() returned. The consumer should have been resumed once.
+  EXPECT_EQ(resume_count, 1);
+
+  // Now satisfy the condition
+  value = 42;
+  event.set();
+  spin_until_done(consumer_task);
+
+  ASSERT_TRUE(consumer_task.handle.done());
+  EXPECT_EQ(resume_count, 2);
+}
+
 TEST_F(EventTest, CancelDoesNotDoubleResume)
 {
   Event event(*ctx_);