Skip to content

Commit 88f355e

Browse files
committed
Use unconditional wait when possible.
Previously the max() value of the steady time was used as the default deadline. In some environments this results in overflows in the underlying pthread_cond_timedwait call, which waits for the conditional variable in the events queue implementation. Consequently, this lead to freezes in the executor. Reducing the deadline significantly helped, but using `cv.wait` instead of `cv_.wait_until` seems to be the cleaner solution. Signed-off-by: Florian Vahl <[email protected]>
1 parent 9695271 commit 88f355e

File tree

3 files changed

+10
-9
lines changed

3 files changed

+10
-9
lines changed

rclpy/src/rclpy/events_executor/events_executor.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ void EventsExecutor::spin(std::optional<double> timeout_sec, bool stop_after_use
176176
const auto timeout_ns = std::chrono::duration_cast<std::chrono::nanoseconds>(
177177
std::chrono::duration<double>(*timeout_sec));
178178
const auto end = std::chrono::steady_clock::now() + timeout_ns;
179-
events_queue_.RunUntil(end);
179+
events_queue_.Run(end);
180180
} else {
181181
events_queue_.Run();
182182
}

rclpy/src/rclpy/events_executor/events_queue.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,15 +29,18 @@ void EventsQueue::Enqueue(std::function<void()> event_handler)
2929
cv_.notify_one();
3030
}
3131

32-
void EventsQueue::Run() {RunUntil(std::chrono::steady_clock::time_point::max());}
33-
34-
void EventsQueue::RunUntil(std::chrono::steady_clock::time_point deadline)
32+
void EventsQueue::Run(const std::optional<std::chrono::steady_clock::time_point> deadline)
3533
{
3634
while (true) {
3735
std::function<void()> handler;
3836
{
3937
std::unique_lock<std::mutex> lock(mutex_);
40-
cv_.wait_until(lock, deadline, [this]() {return stopped_ || !queue_.empty();});
38+
auto pred = [this]() {return stopped_ || !queue_.empty();};
39+
if (deadline) {
40+
cv_.wait_until(lock, *deadline, pred);
41+
} else {
42+
cv_.wait(lock, pred);
43+
}
4144
if (stopped_ || queue_.empty()) {
4245
// We stopped for some reason other than being ready to run something (stopped or timeout)
4346
return;

rclpy/src/rclpy/events_executor/events_queue.hpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include <functional>
2121
#include <mutex>
2222
#include <queue>
23+
#include <optional>
2324

2425
namespace rclpy
2526
{
@@ -37,12 +38,9 @@ class EventsQueue
3738
/// Add an event handler to the queue to be dispatched. Can be invoked by any thread.
3839
void Enqueue(std::function<void()>);
3940

40-
/// Run event handlers indefinitely, until stopped.
41-
void Run();
42-
4341
/// Run all ready event handlers, and any that become ready before the given deadline. Calling
4442
/// Stop() will make this return immediately even if ready handlers are enqueued.
45-
void RunUntil(std::chrono::steady_clock::time_point);
43+
void Run(const std::optional<std::chrono::steady_clock::time_point> = {});
4644

4745
/// Causes any Run*() methods outstanding to return immediately. Can be invoked from any thread.
4846
/// The stopped condition persists (causing any *subsequent* Run*() calls to also return) until

0 commit comments

Comments
 (0)