-
Notifications
You must be signed in to change notification settings - Fork 43
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
Deserialization error when subscribing to /rosout #61
Comments
I am also able to repro this bug using the osrf/ros2:nightly docker image, but not in the amd64/ros:crystal image. That means the bug shows up in master but not in release? |
See a pretty simple repro in my commit above. I've confirmed the repro in the osrf/ros2:nightly docker image, and that it does not repro in the amd64/ros:crystal-ros-core image. Here's the asan output I get.
|
Most recent changes in files related to that trace |
This might help a little. Using the repro I posted above, I bisected using the build artifacts from ci.ros2.org.
|
The pair of commits that expose this bug are However, after looking through them I don't think they actually caused the bug. To recap, we do know that this is only happening with CPP subscriber/publishers and some of the code in the first commit I linked might help point to where things are broken. I setting my investigation aside for a bit. Hopefully this information helps. |
It seems odd that a C++ Subscriber is somehow causing it to use the rosidl_generator_c__String__assign function. From the investigation we've done, it appears the realloc issue is because it's trying to realloc the pointer to the char buffer for the C++ string class. However, the C++ String class is using the short string optimization. So the pointer it is trying to realloc is actually pointing to the middle of the data in the String class, which is why the realloc fails because it's trying to realloc at a location that's in the middle of a previous alloc and not at the start. It would be good to know why it's trying to use C code to update a C++ class. It's not clear to us if this was by design or if this is a bug with it trying to use the wrong typesupport class to deserialize. One theory we had was that the new rosout feature might be registering the C typesupport for the rosout topic and that the Subscriber written in C++ is then using that instead of the CPP typesupport provided during construction. We haven't confirmed that theory though. |
FastRTPS requiers the typesupport to be registered for every participant, this typesupport must be identified by a type_name. When a node is initialized logging_rosout registers the C typessuport but later when the subscriber attempts to register the cpp typesupport it finds that there is a typesupport already registered, (the C typesuppport the logging_rosout has already registered), and therefore it doesn't registers a CPP typesupport and later it attempts to use the C typesupport on a CPP class. This is due to the fact that the type_name generated is identical for C and CPP typesupports. One workaround is to run the subscriber with the __log_disable_rosout:=true option which prevents the logging_rosout publisher creation and thus the registration of the C typesupport implementation for the Log messages. A solutions would be to disambiguate the different typesupports when registering them to Fastrtps, but further investigation is needed. @wjwwood @dirk-thomas Any thoughts? |
The I don't see any reference of a publisher with a C++ typesupport? From where is the C++ message being published? That code likely needs to convert the C++ message into a C message. |
I have tested the same reproduction with |
This is a bug in rmw_fastrtps. It doesn't support to have different typesupports (C and CPP) in the same Participant for the same topic. If the use case is to have a node subscribed to /rosout to listen for log messages a workaround is to disable rosout log in that node by running with the _log_disable_rosout:=true flag. |
Bug report created on ros2/rmw_fastrtps#265 |
Closing in favor of ros2/rmw_fastrtps#265 since the remaining problem is FastRTPS specific. |
Bug report
Required Info:
Operating System:
Ubuntu 16.04 with ROS running inside a docker container
FROM ubuntu:bionic
Installation type:
Build from source following https://index.ros.org/doc/ros2/Installation/Linux-Development-Setup/, using https://raw.githubusercontent.com/ros2/ros2/master/ros2.repos
Version or commit hash:
DDS implementation:
Client library (if applicable):
Steps to reproduce issue
apply the following diff
then build and launch
Expected behavior
In the standard output we see messages from the talker node like
[INFO] [talker]: Publishing: 'Hello World: 35'
, alternating with messagesI heard rcl_interfaces::msg::Log with stamp=[(1548897558, 380492253)], level=...
from the modified listener node, that is listening to rosoutActual behavior
The modified listener node that is listening to rosout has
realloc
errors, that are reported by valgrind in demo_listener.log as followsAdditional information
Feature request
Feature description
Implementation considerations
The text was updated successfully, but these errors were encountered: