rclcpp
rclcpp copied to clipboard
ros2
[Backport]Fix subscription.is_serialized() for callbacks with message info (#1950)
-
Fix subscription.is_serialized() for callbacks with message info argument
-
Add tests + please linters
@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.
@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.)
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.
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.
@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 https://github.com/ros2/rclpy/commit/ad226136bd1675f886dcbc618044714c2edc8871 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.
@roscan-tech we have had discussion in PMC meeting, and we are going to try ABI compatibility checker in rpr job. we believe that is better and consistent process to manage the backports to the downstream distribution than picking up some fixes like this. besides, based on Interim policy on the use of generative AI in OSRF projects we need to refrain from using generative AI tools or bot. So no further PRs generated by AI could be taken. for backports, we will ask the reason and background to the developer and contributor, assess critical severity. we will review https://github.com/ros2/launch_ros/pull/411 and https://github.com/ros2/rclcpp/pull/2622, since those are created before announcement and content of those PRs are already covered and acceptable.
Pulls: ros2/rclcpp#2622 Gist: https://gist.githubusercontent.com/fujitatomoya/c557ed897df99e56237f2f32f759b0e1/raw/cc504a694912d880f15bf8879db36011edc1a872/ros2.repos BUILD args: --packages-above-and-dependencies rclcpp TEST args: --packages-above rclcpp ROS Distro: humble Job: ci_launcher ci_launcher ran: https://ci.ros2.org/job/ci_launcher/14666
- Linux
- Linux-aarch64
- Linux-rhel
- Windows
RHEL failures are unrelated https://ci.ros2.org/job/ci_linux-rhel/1473/cmake/new/ (RTI cmake) and https://ci.ros2.org/job/ci_linux-rhel/1473/testReport/ (python yaml). i will go ahead to merge this.