Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AP_DDS: Race condition generating DDS messages (microxrceddsgen is not thread safe) #28497

Open
Ryanf55 opened this issue Oct 30, 2024 · 1 comment

Comments

@Ryanf55
Copy link
Collaborator

Ryanf55 commented Oct 30, 2024

Bug report

Issue details

Very occasionally, microxrceddsgen fails to generate messages. This is likely because waf is writing to the same file from multiple threads.

Version
4.5
Platform
[ ] All
[ ] AntennaTracker
[ ] Copter
[ ] Plane
[ ] Rover
[ ] Submarine

Airframe type
all
Hardware type
sitl
Logs

Details

microxrceddsgen can take an argument for multiple files at the same time. We call bld() once for each file. It would be faster to do batch them per folder, or all together at the same time.

microxrceddsgen -cs -replace -default-container-prealloc-size 8 -d build/IDL -I libraries/AP_DDS/Idl/ /home/ryan/Dev/ardu_ws/src/ardupilot/libraries/AP_DDS/Idl/ardupilot_msgs/msg/GlobalPosition.idl /home/ryan/Dev/ardu_ws/src/ardupilot/libraries/AP_DDS/Idl/ardupilot_msgs/srv/ArmMotors.idl /home/ryan/Dev/ardu_ws/src/ardupilot/libraries/AP_DDS/Idl/ardupilot_msgs/srv/ModeSwitch.idl /home/ryan/Dev/ardu_ws/src/ardupilot/libraries/AP_DDS/Idl/builtin_interfaces/msg/Duration.idl /home/ryan/Dev/ardu_ws/src/ardupilot/libraries/AP_DDS/Idl/builtin_interfaces/msg/Time.idl /home/ryan/Dev/ardu_ws/src/ardupilot/libraries/AP_DDS/Idl/geographic_msgs/msg/GeoPoint.idl /home/ryan/Dev/ardu_ws/src/ardupilot/libraries/AP_DDS/Idl/geographic_msgs/msg/GeoPointStamped.idl /home/ryan/Dev/ardu_ws/src/ardupilot/libraries/AP_DDS/Idl/geographic_msgs/msg/GeoPose.idl /home/ryan/Dev/ardu_ws/src/ardupilot/libraries/AP_DDS/Idl/geographic_msgs/msg/GeoPoseStamped.idl /home/ryan/Dev/ardu_ws/src/ardupilot/libraries/AP_DDS/Idl/geometry_msgs/msg/Point.idl /home/ryan/Dev/ardu_ws/src/ardupilot/libraries/AP_DDS/Idl/geometry_msgs/msg/Pose.idl /home/ryan/Dev/ardu_ws/src/ardupilot/libraries/AP_DDS/Idl/geometry_msgs/msg/PoseStamped.idl /home/ryan/Dev/ardu_ws/src/ardupilot/libraries/AP_DDS/Idl/geometry_msgs/msg/Quaternion.idl /home/ryan/Dev/ardu_ws/src/ardupilot/libraries/AP_DDS/Idl/geometry_msgs/msg/Transform.idl /home/ryan/Dev/ardu_ws/src/ardupilot/libraries/AP_DDS/Idl/geometry_msgs/msg/TransformStamped.idl /home/ryan/Dev/ardu_ws/src/ardupilot/libraries/AP_DDS/Idl/geometry_msgs/msg/Twist.idl /home/ryan/Dev/ardu_ws/src/ardupilot/libraries/AP_DDS/Idl/geometry_msgs/msg/TwistStamped.idl /home/ryan/Dev/ardu_ws/src/ardupilot/libraries/AP_DDS/Idl/geometry_msgs/msg/Vector3.idl /home/ryan/Dev/ardu_ws/src/ardupilot/libraries/AP_DDS/Idl/geometry_msgs/msg/Vector3Stamped.idl /home/ryan/Dev/ardu_ws/src/ardupilot/libraries/AP_DDS/Idl/rosgraph_msgs/msg/Clock.idl /home/ryan/Dev/ardu_ws/src/ardupilot/libraries/AP_DDS/Idl/sensor_msgs/msg/BatteryState.idl /home/ryan/Dev/ardu_ws/src/ardupilot/libraries/AP_DDS/Idl/sensor_msgs/msg/Imu.idl /home/ryan/Dev/ardu_ws/src/ardupilot/libraries/AP_DDS/Idl/sensor_msgs/msg/Joy.idl /home/ryan/Dev/ardu_ws/src/ardupilot/libraries/AP_DDS/Idl/sensor_msgs/msg/NavSatFix.idl /home/ryan/Dev/ardu_ws/src/ardupilot/libraries/AP_DDS/Idl/sensor_msgs/msg/NavSatStatus.idl /home/ryan/Dev/ardu_ws/src/ardupilot/libraries/AP_DDS/Idl/std_msgs/msg/Header.idl /home/ryan/Dev/ardu_ws/src/ardupilot/libraries/AP_DDS/Idl/tf2_msgs/msg/TFMessage.idl

We'll need to generate them into the right folder too. See eProsima/Micro-XRCE-DDS-Gen#80

A possible workaround is to generate each IDL in it's own folder, or skip dependent files.

Here's gen_cmd on master:

'/home/ryan/Dev/ros2_ws/src/Micro-XRCE-DDS-Gen/scripts/microxrceddsgen -cs -replace -default-container-prealloc-size 8 -d libraries/AP_DDS/generated/ardupilot_msgs/msg -I /home/ryan/Dev/ardu_ws/src/ardupilot/libraries/AP_DDS/Idl /home/ryan/Dev/ardu_ws/src/ardupilot/libraries/AP_DDS/Idl/ardupilot_msgs/msg/GlobalPosition.idl'

And a proposed fix:

'/home/ryan/Dev/ros2_ws/src/Micro-XRCE-DDS-Gen/scripts/microxrceddsgen -cs -replace -default-container-prealloc-size 8 -d libraries/AP_DDS/generated/builtin_interfaces/msg -I /home/ryan/Dev/ardu_ws/src/ardupilot/libraries/AP_DDS/Idl /home/ryan/Dev/ardu_ws/src/ardupilot/libraries/AP_DDS/Idl/builtin_interfaces/msg/Duration.idl /home/ryan/Dev/ardu_ws/src/ardupilot/libraries/AP_DDS/Idl/builtin_interfaces/msg/Time.idl'
@Ryanf55
Copy link
Collaborator Author

Ryanf55 commented Nov 18, 2024

I have a branch that is close to fixing this: 28497-fix-waf-dds-gen-race

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

1 participant