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

[Backport]Fix subscription.is_serialized() for callbacks with message info (#1950) #2622

Open
wants to merge 1 commit into
base: humble
Choose a base branch
from

Conversation

roscan-tech
Copy link

  • Fix subscription.is_serialized() for callbacks with message info argument

  • Add tests + please linters

…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]>
Copy link
Collaborator

@fujitatomoya fujitatomoya left a 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?

@roscan-tech
Copy link
Author

roscan-tech commented Sep 10, 2024

@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 Mergify is an excellent tool for automating merge processes, it still requires manual judgment and activation. Therefore, it might be beneficial to develop an automated bot that can assess whether a fix needs to be synchronized across different versions.

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.

@fujitatomoya
Copy link
Collaborator

@roscan-tech thank you for sharing that.

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.

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?

fixes are not always applied consistently across all of them.

this is true, some fixes are missing even it is ABI compatible. (this PR is an example.)

@clalancette
Copy link
Contributor

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.

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.

@roscan-tech
Copy link
Author

roscan-tech commented Sep 11, 2024

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?

@fujitatomoya Thank you for your attention to and feedback on our work.
In fact, we identified synchronization issues across different versions within ROS 2 and designed a tool to address this. We have tested our tool on the humble, iron, rolling, and foxy distributions and the foxy version had the most cross-version issues.

@roscan-tech
Copy link
Author

roscan-tech commented Sep 11, 2024

@clalancette @fujitatomoya
Thank you for your attention to and feedback on our work. We sincerely apologize for any inconvenience and time we may have caused. We will promptly cease further PR submissions.

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 foxy version. There may be other critical bugs that I have not yet had the opportunity to examine due to time constraints.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants