rcpputils
rcpputils copied to clipboard
ros2
Avoid priority inversions when using FIFO scheduling
The changes in this pull request add two new mutex classes to rclcpp:
- PIMutex
- RecursivePIMutex
These mutex implementations are compatible with the std lib ones, since they derive from them, but they additionally possess the priority inheritance trait. Therefore threads which are using these mutexes cannot suffer from priority inversion when using realtime scheduling, i.e. SCHED_FIFO as defined by the POSIX standard. In general, using mutexes with priority inheritance is a necessary step for making ROS2 more suitable for hard realtime scenarios.
The use of these new mutex implementations is demonstrated in the following pull request for rclcpp: https://github.com/ros2/rclcpp/pull/2078 It can be seen that the necessary code changes are minimal. Usually it is enough to replace std::mutex with rclcpp::PIMutex and std::recursive_mutex with rclcpp::RecursivePIMutex. Only in some cases when lock guards are used it's necessary to explicitly state the desired mutex type, i.e. std::mutex or std::recursive_mutex, to avoid compiler errors.
In general it would be best to avoid the use of mutexes by design through lock free data structures and clever use of atomics, if possible. But refactoring large parts of the ROS2 source code is not really a viable option to make ROS2 more realtime friendly in the short term. The approach with the best cost-benefit ratio presumably is to simply replace all mutex classes as described beforehand.
This pull request contains new units tests. One of the new test cases ensures that priority inversion is avoided when using the new mutex classes. To compile this particular testcase the following pull request for rcutils is needed: https://github.com/ros2/rcutils/pull/406
This pull request has been mentioned on ROS Discourse. There might be relevant details there:
https://discourse.ros.org/t/avoiding-priority-inversions-includes-draft-pull-requests/29219/1
A ROS Discourse article has been created for this pull request: https://discourse.ros.org/t/avoiding-priority-inversions-includes-draft-pull-requests/29219
Looks good to me, but I am not a reviewer. I would really like to have the commits squashed / rebased (i.e. stuff like "code style fix" can easily be fixuped).
@DasRoteSkelett
Looks good to me, but I am not a reviewer. I would really like to have the commits squashed / rebased (i.e. stuff like "code style fix" can easily be fixuped).
That's definitely nicer.
This pull request has been mentioned on ROS Discourse. There might be relevant details there:
https://discourse.ros.org/t/avoiding-priority-inversions-includes-draft-pull-requests/29219/7
This pull request has been mentioned on ROS Discourse. There might be relevant details there:
https://discourse.ros.org/t/avoiding-priority-inversions-includes-draft-pull-requests/29219/8
I added a CMake option in the CMakeLists.txt which allows to turn the priority inheritance feature on or off without changing a single line of code. The changes also improved readability.