Fix bad_function_call by replacing rclcpp::spin_some with SingleThreadedExecutor

[emersonknapp]

Extension of #576 - same fix applied in more places. I have been seeing occasional test flakiness due to this cause. See details from #576 following

Every time we called spin_some, a new executor was constructed. This would happen potentially a few thousand times per test. Given that the global context was not shutdown during this time, it would accumulate on_shutdown_callbacks_, one from each executor that was created. When this list was very large we would occasionally get std::bad_function_call for one of the callbacks in the shutdown.

With this change, the global Context has only 2 items in on_shutdown_callbacks_, and I am not able to reproduce bad_function_call any more.

[clalancette]

Overall, this is a good thing to fix (it's one of the persistent failures in CI).

There's a problem in the code that I pointed out inline; once that is fixed, hopefully the tests will successfully pass.

Separately, this isn't a great way for the rclcpp::spin_some API to behave. This would be hard for others to debug. I have a running list of APIs that I think are "hard" to use; I'll add this to see what we can do to make it less dangerous.

[clalancette]

This improves things, certainly. The tests are still failing with a timeout in test_play__rmw_cyclonedds_cpp; I'm not sure if that is known or new from this PR. I'm going to approve anyway and let you figure out whether this is an improvement regardless of that failing test.

[emersonknapp]

Gist: https://gist.githubusercontent.com/emersonknapp/ac0d6eb824f96d7d6dd8306fdace023b/raw/82faf3109ee0d7fdb8bbc453154b6b7ebd0a5be2/ros2.repos
BUILD args: --packages-up-to rosbag2_test_common rosbag2_transport rosbag2_tests rosbag2
TEST args: --packages-select rosbag2_test_common rosbag2_transport rosbag2_tests rosbag2
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/8074

• Linux
• Linux-aarch64
• macOS
• Windows

[emersonknapp]

This improves things, certainly. The tests are still failing with a timeout in test_play__rmw_cyclonedds_cpp; I'm not sure if that is known or new from this PR.

It was new from this PR - after a bit of debugging I have realized the std::async thread from spin_subscriptions had no actual exit condition, and so was hanging the caller indefinitely. The reason that it worked before was that the Executor constructor in rclcpp::spin_some would throw an exception once rclcpp had been shutdown, thereby killing the thread. I think this was a totally accidental flow control - I've fixed it now by extending the loop check to while (rclcpp::ok() && other_condition)

[emersonknapp]

• Linux
• Linux-aarch64
• macOS
• Windows