Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixup Executor::spin_all() regression fix #2517

Merged
merged 9 commits into from
May 2, 2024
Prev Previous commit
added some more comments about the implementation
Signed-off-by: William Woodall <william@osrfoundation.org>
wjwwood committed May 2, 2024
commit 8587653dc4feb6adce1b4eb0c8c1cb79d687ff90
16 changes: 16 additions & 0 deletions rclcpp/src/rclcpp/executor.cpp
Original file line number Diff line number Diff line change
@@ -373,6 +373,22 @@ Executor::spin_some_impl(std::chrono::nanoseconds max_duration, bool exhaustive)
wait_for_work(std::chrono::milliseconds(0));
bool just_waited = true;

// The logic of this while loop is as follows:
//
// - while not shutdown, and spinning (not canceled), and not max duration reached...
// - try to get an executable item to execute, and execute it if available
// - otherwise, reset the wait result, and ...
// - if there was no work available just after waiting, break the loop unconditionally
// - this is appropriate for both spin_some and spin_all which use this function
// - else if exhaustive = true, then wait for work again
// - this is only used for spin_all and not spin_some
// - else break
// - this only occurs with spin_some
//
// The logic of this loop is subtle and should be carefully changed if at all.
// See also:
// https://github.com/ros2/rclcpp/issues/2508
// https://github.com/ros2/rclcpp/pull/2517
while (rclcpp::ok(context_) && spinning.load() && max_duration_not_elapsed()) {
AnyExecutable any_exec;
if (get_next_ready_executable(any_exec)) {