Skip to content

Commit

Permalink
Only check for new work once in spin_some (#471)
Browse files Browse the repository at this point in the history
To prevent the Executor::spin_some() method for potentially running
indefinitely long, a flag only allows the method which loads new
work to execute once per cycle.

Distribution Statement A; OPSEC #2893

Signed-off-by: Roger Strain <rstrain@swri.org>
  • Loading branch information
Roger Strain committed Nov 18, 2019
1 parent b92db52 commit 870edc8
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 4 deletions.
3 changes: 2 additions & 1 deletion rclcpp/include/rclcpp/executor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,8 @@ class Executor
bool
get_next_executable(
AnyExecutable & any_executable,
std::chrono::nanoseconds timeout = std::chrono::nanoseconds(-1));
std::chrono::nanoseconds timeout = std::chrono::nanoseconds(-1),
bool allow_new_work = true);

/// Spinning state, used to prevent multi threaded calls to spin and to cancel blocking spins.
std::atomic_bool spinning;
Expand Down
8 changes: 5 additions & 3 deletions rclcpp/src/rclcpp/executor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -234,13 +234,15 @@ Executor::spin_some(std::chrono::nanoseconds max_duration)
throw std::runtime_error("spin_some() called while already spinning");
}
RCLCPP_SCOPE_EXIT(this->spinning.store(false); );
bool allow_new_work = true;
while (spinning.load() && max_duration_not_elapsed()) {
AnyExecutable any_exec;
if (get_next_executable(any_exec, std::chrono::milliseconds::zero())) {
if (get_next_executable(any_exec, std::chrono::milliseconds::zero(), allow_new_work)) {
execute_any_executable(any_exec);
} else {
break;
}
allow_new_work = false;
}
}

Expand Down Expand Up @@ -561,14 +563,14 @@ Executor::get_next_ready_executable(AnyExecutable & any_executable)
}

bool
Executor::get_next_executable(AnyExecutable & any_executable, std::chrono::nanoseconds timeout)
Executor::get_next_executable(AnyExecutable & any_executable, std::chrono::nanoseconds timeout, bool allow_new_work)
{
bool success = false;
// Check to see if there are any subscriptions or timers needing service
// TODO(wjwwood): improve run to run efficiency of this function
success = get_next_ready_executable(any_executable);
// If there are none
if (!success) {
if (!success && allow_new_work) {
// Wait for subscriptions or timers to work on
wait_for_work(timeout);
if (!spinning.load()) {
Expand Down

0 comments on commit 870edc8

Please sign in to comment.