Skip to content

fix: prevent double-shutdown in rclcpp::Context - #3216

Closed
fishitiny wants to merge 1 commit into
ros2:humblefrom
fishitiny:fix/context-double-shutdown
Closed

fix: prevent double-shutdown in rclcpp::Context#3216
fishitiny wants to merge 1 commit into
ros2:humblefrom
fishitiny:fix/context-double-shutdown

Conversation

@fishitiny

@fishitiny fishitiny commented Aug 5, 2026

Copy link
Copy Markdown

Description

The signal handler thread (deferred_signal_handler) and the main thread may both call Context::shutdown() during Ctrl-C, causing a double-free or heap corruption in the underlying rcl_context_t and glibc.

Add an atomic flag is_shutting_down_ to ensure only the first call to shutdown() proceeds; subsequent calls return immediately.

Steps to reproduce:

  1. Launch any ROS 2 node with shutdown_on_signal=true
  2. Press Ctrl-C
  3. Observe SIGABRT in rclcpp::Context::shutdown() called from rclcpp::SignalHandler::deferred_signal_handler()

GDB backtrace from core dump:
Thread 1: raise() -> abort()
-> rclcpp::Context::shutdown()
-> rclcpp::SignalHandler::deferred_signal_handler()

Is this user-facing behavior change?

Did you use Generative AI?

no

The signal handler thread (deferred_signal_handler) and the main thread
may both call Context::shutdown() during Ctrl-C, causing a double-free
or heap corruption in the underlying rcl_context_t and glibc.

Add an atomic flag is_shutting_down_ to ensure only the first call to
shutdown() proceeds; subsequent calls return immediately.

Steps to reproduce:
1. Launch any ROS 2 node with shutdown_on_signal=true
2. Press Ctrl-C
3. Observe SIGABRT in rclcpp::Context::shutdown() called from
   rclcpp::SignalHandler::deferred_signal_handler()

GDB backtrace from core dump:
  Thread 1: raise() -> abort()
    -> rclcpp::Context::shutdown()
    -> rclcpp::SignalHandler::deferred_signal_handler()

Signed-off-by: fishitiny <fishitiny@users.noreply.github.com>
@fishitiny
fishitiny marked this pull request as draft August 5, 2026 10:56
@fishitiny
fishitiny marked this pull request as ready for review August 5, 2026 10:58
@mergify

mergify Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@jmachowinski

Copy link
Copy Markdown
Collaborator

This is ABI breaking. Therefore it can not be merged into a stable distro.
This needs also to be fixed in rolling first, if it was not fixed already.

@fujitatomoya

Copy link
Copy Markdown
Collaborator

@fishitiny thanks for rasing the issue here, as @jmachowinski mentioned above, can you address this issue to our development branch which is rolling? besides, ABI breaking change is not allowed to downstream distribution. that said it will not be possible to backport the current fix to the humble.

@fujitatomoya

Copy link
Copy Markdown
Collaborator

@fishitiny if you can take a look at #3217, thta would be appreciated.

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