thread-safe implementation in realtime_publisher

[Maverobot]

When reading the code, I have a few questions regarding realtime_publisher.h

1. volatile bool is used for cross-threads flags. As far as I know, it is undefined behavior in C++. Should we replace that with std::atomic<bool>?
   Some related discussions:
   • C++ : std::atomic and volatile bool
   • Is volatile bool for thread control considered wrong?
2. int turn_ is also being used in both threads and not explicitly guarded by any mutex.
3. Why is NO_POLLING not activated by default? Does it mean the current release version is actually using polling instead of condition_variable, which is in my opinion a much better choice?

[matthew-reynolds]

1. Indeed, volatile bool here is incorrect and dangerous. Looks like something written nearly 10 years ago that's been carried forward ever since. I don't think the solution is atomic<bool> though, I think atomic_flag is more suited since it is guaranteed to be implemented lock-free, while the atomic<T> types may be implemented with locks. (Practically, I imagine all implementation for an atomic<bool> are lock-free, but we should still strive for correctness.)
   Looks like Use std::atomic instead of volatile #51 was just opened to address this issue - Let's move this discussion there?

2. Agreed, int turn_ also looks like a good candidate for an std::atomic<> type, this time std::atomic<int>.

3. I don't know why NON_POLLING isn't default (@bmagyar thoughts? My only guess is that when it was written in 2013, it required boost and was therefore "heavier"?), but from Fix building realtime_publisher using NON_POLLING #43 it looks like it currently doesn't build anyways. Perhaps once it's fixed we should investigate enabling it by default, at least in Noetic.

[bmagyar]

Thank you guys for chasing this.

I propose the following:

• Fix building realtime_publisher using NON_POLLING #43 to fix the state of affairs in Melodic
• Use atomic types #52 to bring a proper fix and to use the non-polling in Noetic
• Use std::atomic instead of volatile #51 to spice up the ROS2 world, here I'd prefer to not disturb Eloquent but rather target Foxy. First and foremost, I'll update the master branch to host the latest ros2 branch and retarget Use std::atomic instead of volatile #51 to that, then cut a foxy release

[christophfroehlich]

Moving the last open point to #199