ros-controls / ros-controls/ros2_controllers

[JointTrajectoryController] Avoid potential race conditions

Open
#166 3 comments 0 reactions 1 assignee View on GitHub

@matthew-reynolds is already working on this.

Since Apr 13, 2021.

help wanted high-priority persistent
Dominant language
C++
Stars
834
Forks
530
Avg merge
18h 3m
Merged PRs (30d)
19

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 previous rt_active_goal_ has pending operations, and thus dropping those operations. I think we should call goal_handle_timer_::execute_callback() or rt_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 new rt_goal, and we could end up losing the goal handle. This would involve a little more work to solve, for example only resetting the rt_active_goal_ if new_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

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.