There’s been some discussion about adding clang’s compile time thread safety analysis for ROS2 core packages, to help us find mistakes automatically and early on. I’ve talked to @tfoote and @Thomas_Moulard about this a bit offline, we wanted to move it into a public forum before moving further.
The thread safety features are enabled by some annotations that are best used by macros that can be turned off when using non-clang compilers (see mutex.h in the clang doc page), as a general utility. The first question is, where should these be defined?
- One thought was to add it to
rclcpp, but since we definitely want to use it in the
rmwlayer, that would be too high
- Another candidate is
rcutils, but these macros are C++ specific (extending functionality of
std::mutex, and that packages is meant to be C-specific
Given that, I personally think creating a new
rcpputils package makes the most sense. It hasn’t been needed yet but could make sense now given we have something that would fit there.
In terms of using these annotations, we have a Ci.ros2.org clang nightly job now available that could raise thread safety warnings for annotated code, at least in the nightly build.
For usage and roll-out, the
rmw packages are prime candidates for first packages to start using the annotations, since they are highly threaded code. The thinking on guidelines
- private mutexes
- use GUARDED_BY on mutex-protected members to surface the rest of the usage needs
- copy small things to avoid blocking
I’ve put together a proof of concept in
rmw_fastrtps to illustrate
Before changing anything, want to hear thoughts on 1) where the macros should live, whether we should create that new package, and 2) usage patterns as put together in that WIP commit