Fix thread safety in LifecycleNode::get_current_state() for Humble

[schornakj]

Implements #2172.

• Backports the changes to LifecycleNodeInterfaceImpl from Bugfix 20210810 get current state #1756. It's OK to apply these changes to Humble because they do not modify the public ABI.
• Improve thread safety when using State::state_handle_:
  • Add a new MutexMap class to associate a mutex with each instance of State.
  • Add a private static MutexMap member to State. This type of change is allowed under REP-0009.
• Add a unit test based on the example code from rclcpp_lifecycle::LifecycleNode::get_current_state is not thread safe #1746 which fails without the above changes.

[schornakj]

@clalancette @fujitatomoya what does the review process and timeline look like for getting this in? I noticed that this PR was added to the Humble Patch Release 4 project.

[fujitatomoya]

@schornakj generally we are now working on the Iron Irwini Release Test for now. but i will try to find some time to do review. let me have a week or two to get back to you. if anyone else are willing to do community review, that would be really appreciated.

[fujitatomoya]

a few minor comments, overall lgtm.

[fujitatomoya]

lgtm

[fujitatomoya]

@clalancette @ros2/team can I have 2nd review on this before CI?

[schornakj]

@fujitatomoya just checking in, were you able to find another person from the ROS 2 team to give this a second review?

[fujitatomoya]

No not yet, but i do need a second review on this.

[schornakj]

@clalancette do you have some time to review this PR? It would be really great to be able to fix this thread safety issue at the root level. While I've been able to patch or work around this problem in some packages that use rclcpp_lifecycle (for example, by reverting a backported feature in ros-controls/ros2_control#1015), I keep running into new and exciting ways in which this bug causes my robots to crash.

[clalancette]

In short, this is a fantastic backport. Thanks for taking the time to do this.

I've left three really small items to fix. Once those are fixed, I'll go ahead and run CI on this.

[schornakj]

@clalancette thank you very much for the review! I just pushed up a commit adding the includes you pointed out.

[clalancette]

CI:

• Linux
• Linux-aarch64
• Windows

[schornakj]

CI failed with an error building rosbag2_transport, which I didn't modify as part of my changes:

--- stderr: rosbag2_transport
17:32:17 �[K/home/jenkins-agent/workspace/ci_linux/ws/src/ros2/rosbag2/rosbag2_transport/src/rosbag2_transport/recorder.c�[K In member functio�[Kvoid rosbag2_transport::Recorder::topics_discover�[K’:
17:32:17 �[K/home/jenkins-agent/workspace/ci_linux/ws/src/ros2/rosbag2/rosbag2_transport/src/rosbag2_transport/recorder.cpp:276:��[Kerro��[Kusing element_type = class rclcpp::Cl�[K’ {ak�[Kclass rclcpp::Cl�[K’} has no member name�[Kwait_until_star�[K’
17:32:17   276 |       if (this->get_clock()->�[Kwait_until_star�[K(record_options_.topic_polling_interval)) {
17:32:17       |                              �[K^~~~~~~~~~~~~~~�[K
17:32:17 �[K/home/jenkins-agent/workspace/ci_linux/ws/src/ros2/rosbag2/rosbag2_transport/src/rosbag2_transport/recorder.cpp:280:��[Kerro��[Kusing element_type = class rclcpp::Cl�[K’ {ak�[Kclass rclcpp::Cl�[K’} has no member name�[Kstar�[K’
17:32:17   280 |     if (this->get_clock()->�[Kstar�[K()) {
17:32:17       |                            �[K^~~~�[K
17:32:17 gmake[2]: *** [CMakeFiles/rosbag2_transport.dir/build.make:132: CMakeFiles/rosbag2_transport.dir/src/rosbag2_transport/recorder.cpp.o] Error 1
17:32:17 gmake[1]: *** [CMakeFiles/Makefile2:300: CMakeFiles/rosbag2_transport.dir/all] Error 2
17:32:17 gmake: *** [Makefile:146: all] Error 2
17:32:17 ---
17:32:17 Failed   <<< rosbag2_transport [46.7s, exited with code 2]

Should I rebase my PR branch onto the latest state of ros2:humble? I suspect that my PR branch might just be out of date.

[clalancette]

Ah right. This is failing to pass CI because it needs a rebase. I've done that and pushed it now, here is CI again:

• Linux
• Linux-aarch64
• Windows

[clalancette]

All right, this looks good. Going ahead and merging. We just missed the window for a humble sync, so this will go out with the next patch release (probably 4-6 weeks).

[schornakj]

Awesome!! Thank you for your guidance and help with this fix!

[clalancette]

Thanks for your patience here and for doing the hard work of the backport.