[JointTrajectoryController] Avoid potential race conditions
#166 opened on Apr 7, 2021
Repository metrics
- Stars
- (810 stars)
- PR merge metrics
- (PR metrics pending)
Description
Check the following comment by @matthew-reynolds:
I think this needs further work, probably in a follow-up PR.
In both the old and new code, I worry that we might end up in a condition where we're overwriting the
goal_handle_timer_while the previousrt_active_goal_has pending operations, and thus dropping those operations. I think we should callgoal_handle_timer_::execute_callback()orrt_active_goal_.readFromNonRT()->runNonRealtime()before creating the new timer.Similarly, I think we could probably end up in a race condition where something is resetting the
rt_active_goal_right after this function writes the newrt_goal, and we could end up losing the goal handle. This would involve a little more work to solve, for example only resetting thert_active_goal_ifnew_data_available_ == false. That variable is private, but something along those lines.Would appreciate your thoughts so we can consider appropriate follow-up PRs. This PR leaves the behaviour unchanged from before, and I haven't yet run into either of those cases, so I think we're ok to leave it to a future task.
Originally posted by @matthew-reynolds in https://github.com/ros-controls/ros2_controllers/pull/164#discussion_r606884831