Skip to content

Conversation

@fujitatomoya
Copy link
Collaborator

@fujitatomoya fujitatomoya commented Nov 24, 2025

Description

Backport of #2986

I tried to think of the ABI compatible way to backport the #2986, because it is critical issue.
But ended up removing the I/O (including possible rcl_publish) from the signal handler.

  • reuse signal_received_ as std::atomic_int to store the signal number. this also changes the memory layout/alighment from 1 byte to 4 bytes. checked with amd64 and arm64 platform, this is also gonna be the breaking ABI change, and not guaranteed by standard.
  • external data structure such as code space static variable or per-instance state using instance IDs or pointers, avoiding class member changes. However, this adds complexity and potential thread-safety concerns.

Note

this requires ros2/system_tests#580

Is this user-facing behavior change?

Yes, the user application cannot expect the logging message (or publish) via signal handler.

Did you use Generative AI?

No,

Additional Information

@fujitatomoya fujitatomoya changed the base branch from rolling to kilted November 24, 2025 23:57
Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
@fujitatomoya fujitatomoya force-pushed the fujitatomoya/pull/2986/kilted/backport branch from 58c4c40 to fecd6f5 Compare November 24, 2025 23:58
@fujitatomoya
Copy link
Collaborator Author

@jmachowinski do we have other options to backport #2986? i tried to think of the possible approach, but ended up having this PR instead.

@fujitatomoya
Copy link
Collaborator Author

Pulls: #3000, ros2/system_tests#580
Gist: https://gist.githubusercontent.com/fujitatomoya/cb0cb9f2ccbaa3aacf702208a0af568e/raw/8ebcd90eb3c0a40947db66d0fff6f5af5e62ec85/ros2.repos
BUILD args: --packages-above-and-dependencies rclcpp test_rclcpp
TEST args: --packages-above rclcpp test_rclcpp
ROS Distro: kilted
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/17600

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can move the info output here

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.

3 participants