-
Notifications
You must be signed in to change notification settings - Fork 117
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
Capture std::bad_alloc
on deserializeROSmessage.
#665
Capture std::bad_alloc
on deserializeROSmessage.
#665
Conversation
cannot get the result from server. root@tomoyafujita:~/ros2_ws/colcon_ws# RMW_IMPLEMENTATION=rmw_cyclonedds_cpp ros2 run demo_nodes_cpp add_two_ints_client
^C[INFO] [1674755622.413766599] [rclcpp]: signal_handler(signum=2)
[ERROR] [1674755622.414263025] [add_two_ints_client]: Interrupted while waiting for response. Exiting.
in this sample, cannot observe the core dump or crash. but it is obvious that de-serialization works unexpectedly. root@tomoyafujita:~/ros2_ws/colcon_ws# cat DEFAULT_FASTRTPS_PROFILES.xml
<?xml version="1.0" encoding="UTF-8" ?>
<profiles xmlns="http://www.eprosima.com/XMLSchemas/fastRTPS_Profiles">
<!-- Default publisher profile -->
<data_writer profile_name="default publisher profile" is_default_profile="true">
<qos>
<data_sharing>
<kind>AUTOMATIC</kind>
</data_sharing>
</qos>
<historyMemoryPolicy>DYNAMIC</historyMemoryPolicy>
</data_writer>
<data_reader profile_name="default subscription profile" is_default_profile="true">
<qos>
<data_sharing>
<kind>AUTOMATIC</kind>
</data_sharing>
</qos>
<historyMemoryPolicy>DYNAMIC</historyMemoryPolicy>
</data_reader>
</profiles>
root@tomoyafujita:~/ros2_ws/colcon_ws# export RMW_FASTRTPS_USE_QOS_FROM_XML=1
root@tomoyafujita:~/ros2_ws/colcon_ws# RMW_IMPLEMENTATION=rmw_fastrtps_cpp ros2 run demo_nodes_cpp add_two_ints_server
[INFO] [1674755608.560241436] [add_two_ints_server]: Incoming request
a: -2268473885457029753 b: 1
^C[INFO] [1674755623.645151231] [rclcpp]: signal_handler(signum=2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, this should avoid unexpected exceptions during de-serialization.
@alsora @mauropasse if possible, would you mind trying this patch to see if it can avoid the program crash? (just sending request to the server can get the program crashed, for me that is the security issue...) |
@@ -119,6 +119,11 @@ bool TypeSupport::deserializeROSmessage( | |||
"Fast CDR exception deserializing message of type %s.", | |||
getName()); | |||
return false; | |||
} catch (...) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally, we would prefer to have specific exceptions listed here, rather than just catch everything.
What exceptions can this deserialization throw?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at the original report, it looks like we should just catch std::bad_alloc
for now. @MiguelCompany if you make that change and rebase this PR onto the latest, I'll be happy to approve and run CI on it.
This fixes #733 if I build this branch from source in the humble container. Is there any chance that this gets merged and backported to humble? |
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
5fa730e
to
c4f869a
Compare
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
c4f869a
to
a479bcd
Compare
std::bad_alloc
on deserializeROSmessage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two more minor changes, then we can run CI on this. Thanks!
rmw_fastrtps_dynamic_cpp/include/rmw_fastrtps_dynamic_cpp/TypeSupport_impl.hpp
Outdated
Show resolved
Hide resolved
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Will run CI on this next.
@clalancette would you mind backporting this to iron and humble as well? This would fix #733 then. |
@Mergifyio backport humble iron |
✅ Backports have been created
|
* Capture bad_alloc on deserializeROSmessage. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> (cherry picked from commit 4d0be32)
* Capture bad_alloc on deserializeROSmessage. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> (cherry picked from commit 4d0be32)
* Capture bad_alloc on deserializeROSmessage. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> (cherry picked from commit 4d0be32) Co-authored-by: Miguel Company <miguelcompany@eprosima.com>
* Capture bad_alloc on deserializeROSmessage. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> (cherry picked from commit 4d0be32) Co-authored-by: Miguel Company <miguelcompany@eprosima.com>
* Capture bad_alloc on deserializeROSmessage. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> (cherry picked from commit 4d0be32) Co-authored-by: Miguel Company <miguelcompany@eprosima.com>
hi can i get some help |
@naorwaiss foxy is E.O.L https://docs.ros.org/en/rolling/Releases.html#list-of-distributions, can you try ROS Iron instead of |
um i use jetson nano so i can run ubuntu 20.04 - this mean if i understand it .. that i can install only ros2 foxy because ros2 iron is available to ubuntu 22.04.. |
I think that is the requirement for jetson nano platform? can you ask help for jetson community? |
hi the installation of humble isnt work - can help me to make the bridge between ros2 iron and foxy ? |
cross-distribution communication is not officially supported. |
This should fix crashes coming from serialization mismatches that cannot be detected by Fast CDR
Related to ros2/ros2_documentation#3288 (comment)