[ros-users] [Discourse.ros.org] [Next Generation ROS] Adding clang thread safety analysis for ROS2 core packages
Emerson Knapp via Discourse.ros.org
ros.discourse at gmail.com
Thu Feb 21 00:05:12 UTC 2019
Hi all :wave:
There's been some discussion about adding [clang's compile time thread safety analysis](https://clang.llvm.org/docs/ThreadSafetyAnalysis.html) 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.
### Annotation Macros
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](https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#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 `rmw` layer, 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 https://discourse.ros.org/t/ci-ros2-org-clang-nightly-job-now-available/7857 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
[Visit Topic](https://discourse.ros.org/t/adding-clang-thread-safety-analysis-for-ros2-core-packages/7930/1) or reply to this email to respond.
More information about the ros-users