niklas/rclcpp
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Guard against making multiple result requests for a goal handle (#808)

Browse source 
This fixes a runtime error caused by a race condition when making consecutive requests for the
result.
Specifically, this happens if the user provides a result callback when sending a goal and then
calls async_get_result shortly after.

Resolves #783

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
This commit is contained in:
Jacob Perron 2019-08-07 09:26:19 -07:00 • committed by GitHub
parent 871c375da7
commit 41a99b64d3
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 10 additions and 7 deletions
Download patch file Download diff file Expand all files Collapse all files

10
rclcpp_action/include/rclcpp_action/client.hpp
View file

@ -410,10 +410,7 @@ public:
// This will override any previously registered callback // This will override any previously registered callback
goal_handle->set_result_callback(result_callback); goal_handle->set_result_callback(result_callback);
} }
// If the user chose to ignore the result before, then ask the server for the result now. this->make_result_aware(goal_handle);
if (!goal_handle->is_result_aware()) {
this->make_result_aware(goal_handle);
}
return goal_handle->async_result(); return goal_handle->async_result();
} }
@ -595,6 +592,10 @@ private:
void void
make_result_aware(typename GoalHandle::SharedPtr goal_handle) make_result_aware(typename GoalHandle::SharedPtr goal_handle)
{ {
// Avoid making more than one request
if (goal_handle->set_result_awareness(true)) {
return;
}
using GoalResultRequest = typename ActionT::Impl::GetResultService::Request; using GoalResultRequest = typename ActionT::Impl::GetResultService::Request;
auto goal_result_request = std::make_shared<GoalResultRequest>(); auto goal_result_request = std::make_shared<GoalResultRequest>();
goal_result_request->goal_id.uuid = goal_handle->get_goal_id(); goal_result_request->goal_id.uuid = goal_handle->get_goal_id();
@ -614,7 +615,6 @@ private:
std::lock_guard<std::mutex> lock(goal_handles_mutex_); std::lock_guard<std::mutex> lock(goal_handles_mutex_);
goal_handles_.erase(goal_handle->get_goal_id()); goal_handles_.erase(goal_handle->get_goal_id());
}); });
goal_handle->set_result_awareness(true);
} }
/// \internal /// \internal

3
rclcpp_action/include/rclcpp_action/client_goal_handle.hpp
View file

@ -134,7 +134,8 @@ private:
typename ClientGoalHandle<ActionT>::SharedPtr shared_this, typename ClientGoalHandle<ActionT>::SharedPtr shared_this,
typename std::shared_ptr<const Feedback> feedback_message); typename std::shared_ptr<const Feedback> feedback_message);
void /// Returns the previous value of awareness
bool
set_result_awareness(bool awareness); set_result_awareness(bool awareness);
void void

4
rclcpp_action/include/rclcpp_action/client_goal_handle_impl.hpp
View file

@ -127,11 +127,13 @@ ClientGoalHandle<ActionT>::is_result_aware()
} }
template<typename ActionT> template<typename ActionT>
void bool
ClientGoalHandle<ActionT>::set_result_awareness(bool awareness) ClientGoalHandle<ActionT>::set_result_awareness(bool awareness)
{ {
std::lock_guard<std::mutex> guard(handle_mutex_); std::lock_guard<std::mutex> guard(handle_mutex_);
bool previous = is_result_aware_;
is_result_aware_ = awareness; is_result_aware_ = awareness;
return previous;
} }
template<typename ActionT> template<typename ActionT>