-
Notifications
You must be signed in to change notification settings - Fork 413
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
[Backport]Fix subscription.is_serialized() for callbacks with message info (#1950) #2622
base: humble
Are you sure you want to change the base?
Conversation
…2#1950) * Fix subscription.is_serialized() for callbacks with message info argument Signed-off-by: Ivan Santiago Paunovic <[email protected]> * Add tests + please linters Signed-off-by: Ivan Santiago Paunovic <[email protected]> Signed-off-by: roscan-tech <[email protected]>
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.
I think we can backport this.
@roscan-tech it seems that you are trying to cherry-pick some fixes from the main to humble. Do you have any specific scope or requirement for doing that?
@fujitatomoya Thank you for your attention to my pull requests! Actually, we are working on a research project aimd at improving sotware reliability for ROS2 systems. In particular, we are exploring a new, automatic approach to identify cross-version(distribution/branch) discripencies related to bug-fix. As we know, ROS2 has multiple distributions, but security fixes are not always applied consistently across all of them. What we are doing is to automatically identify these bugs and vulnerabilities to ensure that the fixes are synchronized across different versions. While The potential bugs we currently raised are partically the output of an automated tool, with manual verificaiton. We sincerely appricate your effors helping us confiming these issuse. |
@roscan-tech thank you for sharing that.
The reason that you picked ROS 2 is that you guys use ROS 2 (probably humble) for robot application? Or main purpose is to test your automation tool with ROS 2?
this is true, some fixes are missing even it is ABI compatible. (this PR is an example.) |
So the problem with what we've seen so far as that none of these are security fixes (or, at least, they are not clearly security fixes). Further, just having automation backport things is causing a bunch of work on us, and for no clear benefit. Yes, there are probably bugs in the older distributions, but we have a very small team working on this. Putting additional load on that group to review these automated backports actually makes life worse for our users, because we can't prioritize the things that actually matter (i.e. bugs that real users are running into). Thus, my opinion on this is that we should not do this kind of automated backporting. This is not the last word, and I want to hear opinions from other people on @ros2/team, but I'd appreciate it if you paused opening more of these until the ROS PMC makes a decision regarding these. |
@fujitatomoya Thank you for your attention to and feedback on our work. |
@clalancette @fujitatomoya Our primary objective is to enhance the reliability and consistency of ROS 2 distributions by identifying and synchronizing fixes across different versions. While the fixes we have submitted may not strictly fall under the category of critical security patches, we believe they significantly contribute to system stability and user experience. Our intention is not to impose unnecessary burdens but to assist in making ROS 2 more robust and secure. We are committed to refining our methods and tools to focus on identifying and submitting only the most critical fixes. We would greatly appreciate collaboration from the community in developing such tools together and believe that automating the process of identifying and synchronizing fixes will ultimately benefit the community by ensuring that essential patches are not overlooked in any distribution. If a dedicated team is already addressing this issue, our automation tool could be valuable. While it may add some short-term burden, an effective automated tool can help reduce the workload of the team in the long term. Given that ROS 2 is relatively new and evolving rapidly, the likelihood of synchronization bugs is likely to increase over time. Therefore, such automation tools seem necessary for sustainable development. We will pause further submissions until the ROS PMC reaches a decision. Meanwhile, we are keen to continue the discussion and find a balanced approach that aligns with the community’s needs. Most of the fixes I submitted target commits previously labeled as bugs, which might have led to some misunderstandings. However, I still believe that these fixes warrant backporting. For example, the bug described in ros2/rclpy@ad22613 appears to be quite significant and remains unresolved in the Thank you again for your valuable input and patience. We look forward to continuing the dialogue with the community to find a balanced solution to these issues. |
Fix subscription.is_serialized() for callbacks with message info argument
Add tests + please linters