Skip to content

remove I/O from signal handler.#3000

Merged
ahcorde merged 2 commits intokiltedfrom
fujitatomoya/pull/2986/kilted/backport
Dec 4, 2025
Merged

remove I/O from signal handler.#3000
ahcorde merged 2 commits intokiltedfrom
fujitatomoya/pull/2986/kilted/backport

Conversation

@fujitatomoya
Copy link
Copy Markdown
Collaborator

@fujitatomoya fujitatomoya commented Nov 24, 2025 •
edited
Loading

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
@fujitatomoya
remove I/O from signal handler.
fecd6f5 
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
Copy Markdown
Collaborator Author

fujitatomoya commented Nov 25, 2025

@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
Copy Markdown
Collaborator Author

fujitatomoya commented Nov 25, 2025

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

@fujitatomoya fujitatomoya mentioned this pull request Nov 25, 2025
Merged
jmachowinski
jmachowinski reviewed Nov 25, 2025
View reviewed changes
@fujitatomoya
Copy link
Copy Markdown
Collaborator Author

fujitatomoya commented Dec 1, 2025

@jmachowinski friendly ping

jmachowinski
jmachowinski requested changes Dec 2, 2025
View reviewed changes
@fujitatomoya
Copy link
Copy Markdown
Collaborator Author

fujitatomoya commented Dec 2, 2025

No worries, thanks for coming back 👍

Ah yeah that is what i though too, that we can do at least. i will add it and start the CI.

@fujitatomoya
print either SIGINT or SIGTERM has been received.
df32403 
Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
@fujitatomoya
Copy link
Copy Markdown
Collaborator Author

fujitatomoya commented Dec 2, 2025

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

@fujitatomoya
Copy link
Copy Markdown
Collaborator Author

fujitatomoya commented Dec 4, 2025

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

@ahcorde ahcorde merged commit 3ce946f into kilted Dec 4, 2025
2 of 3 checks passed
@ahcorde ahcorde deleted the fujitatomoya/pull/2986/kilted/backport branch December 4, 2025 09:00
@fujitatomoya
Copy link
Copy Markdown
Collaborator Author

fujitatomoya commented Dec 4, 2025

@Mergifyio backport jazzy humble

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Dec 4, 2025 •
edited
Loading

backport jazzy humble

✅ Backports have been created

Details

mergify bot pushed a commit that referenced this pull request Dec 4, 2025
@fujitatomoya @mergify
remove I/O from signal handler. (#3000)
eed4415 
Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
(cherry picked from commit 3ce946f)
mergify bot pushed a commit that referenced this pull request Dec 4, 2025
@fujitatomoya @mergify
remove I/O from signal handler. (#3000)
88b8438 
Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
(cherry picked from commit 3ce946f)
This was referenced Dec 4, 2025
Merged
Merged
ahcorde pushed a commit that referenced this pull request Dec 5, 2025
@mergify @fujitatomoya
remove I/O from signal handler. (#3000) (#3004)
27edfe1 
(cherry picked from commit 3ce946f)

Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
fujitatomoya added a commit that referenced this pull request Dec 8, 2025
@mergify @fujitatomoya
remove I/O from signal handler. (#3000) (#3005)
2ba0b01 
(cherry picked from commit 3ce946f)

Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ahcorde ahcorde ahcorde approved these changes

@jmachowinski jmachowinski jmachowinski approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@fujitatomoya @ahcorde @jmachowinski