Skip to content

Commit 9387a61

Browse files
authored
Fix Event::set() re-entrant infinite loop (#37)
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.
1 parent 71eb280 commit 9387a61

2 files changed

Lines changed: 51 additions & 3 deletions

File tree

rclcpp_async/include/rclcpp_async/co_context.hpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -516,9 +516,11 @@ inline void Event::WaitAwaiter::await_suspend(std::coroutine_handle<> h)
516516
inline void Event::set()
517517
{
518518
set_ = true;
519-
while (!waiters_.empty()) {
520-
auto w = waiters_.front();
521-
waiters_.pop();
519+
std::queue<Waiter> pending;
520+
pending.swap(waiters_);
521+
while (!pending.empty()) {
522+
auto w = pending.front();
523+
pending.pop();
522524
if (*w.active) {
523525
*w.active = false;
524526
ctx_.resume(w.handle);

rclcpp_async/test/test_event.cpp

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,52 @@ TEST_F(EventTest, CancelDuringWait)
187187
EXPECT_TRUE(was_cancelled);
188188
}
189189

190+
TEST_F(EventTest, SetDoesNotLoopWhenWaiterReregisters)
191+
{
192+
// Reproduces the re-entrant infinite loop bug:
193+
// 1. set() resumes a waiter synchronously
194+
// 2. The waiter checks a condition, finds it unsatisfied, calls clear() + wait()
195+
// 3. wait() re-registers a waiter in waiters_
196+
// 4. set() sees waiters_ is non-empty and loops forever
197+
198+
Event event(*ctx_);
199+
int value = 0;
200+
int resume_count = 0;
201+
202+
auto consumer = [&]() -> Task<void> {
203+
while (value != 42) {
204+
event.clear();
205+
co_await event.wait();
206+
resume_count++;
207+
}
208+
};
209+
210+
auto consumer_task = ctx_->create_task(consumer());
211+
212+
// Spin to let consumer suspend on wait()
213+
for (int i = 0; i < 10; i++) {
214+
executor_.spin_some();
215+
std::this_thread::sleep_for(1ms);
216+
}
217+
EXPECT_EQ(resume_count, 0);
218+
219+
// set() with wrong value — consumer will clear + re-wait.
220+
// With the bug, this never returns (infinite loop).
221+
value = 1;
222+
event.set();
223+
224+
// If we get here, set() returned. The consumer should have been resumed once.
225+
EXPECT_EQ(resume_count, 1);
226+
227+
// Now satisfy the condition
228+
value = 42;
229+
event.set();
230+
spin_until_done(consumer_task);
231+
232+
ASSERT_TRUE(consumer_task.handle.done());
233+
EXPECT_EQ(resume_count, 2);
234+
}
235+
190236
TEST_F(EventTest, CancelDoesNotDoubleResume)
191237
{
192238
Event event(*ctx_);

0 commit comments

Comments
 (0)