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

Fixes #117

Closed

Fixes #117

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion rclcpp/include/rclcpp/executors/events_executor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ class EventsExecutor : public rclcpp::Executor

/// Default destrcutor.
RCLCPP_PUBLIC
virtual ~EventsExecutor() = default;
virtual ~EventsExecutor();

/// Events executor implementation of spin.
/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,12 @@ class EventsExecutorNotifyWaitable final : public EventWaitable

// Destructor
RCLCPP_PUBLIC
virtual ~EventsExecutorNotifyWaitable() = default;
virtual ~EventsExecutorNotifyWaitable()
{
for (auto & gc : notify_guard_conditions_) {
gc->set_on_trigger_callback(nullptr);
}
}

// The function is a no-op, since we only care of waking up the executor
RCLCPP_PUBLIC
Expand Down
6 changes: 6 additions & 0 deletions rclcpp/include/rclcpp/executors/timers_manager.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,12 @@ class TimersManager
*/
std::chrono::nanoseconds get_head_timeout();

/**
* @brief Function to check if the timers thread is running
* @return true if timers thread is running
*/
bool is_running();

private:
RCLCPP_DISABLE_COPY(TimersManager)

Expand Down
20 changes: 20 additions & 0 deletions rclcpp/src/rclcpp/executors/events_executor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,26 @@ EventsExecutor::EventsExecutor(
entities_collector_->add_waitable(executor_notifier_);
}

EventsExecutor::~EventsExecutor()
{
// Set 'spinning' to false and get previous value
auto executor_was_spinning = spinning.exchange(false);

if (executor_was_spinning) {
// Trigger the shutdown guard condition.
// This way, the 'events_queue_' will wake up since a new event has arrived.
// Then since 'spinning' is now false, the spin loop will exit.
shutdown_guard_condition_->trigger();

// The timers manager thread is stopped at the end of spin().
// We have to wait for timers manager thread to exit, otherwise
// the 'timers_manager_' will be destroyed while still being used on spin().
while (timers_manager_->is_running()) {
std::this_thread::sleep_for(1ms);
}
}
}

void
EventsExecutor::spin()
{
Expand Down
5 changes: 5 additions & 0 deletions rclcpp/src/rclcpp/executors/timers_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -284,3 +284,8 @@ void TimersManager::remove_timer(TimerPtr timer)

timer->clear_on_reset_callback();
}

bool TimersManager::is_running()
{
return running_;
}