Skip to content

Fix lock order inversion between GraphListener and Context (ABI compatible backport) (backport #3239) - #3241

Merged
skyegalaxy merged 1 commit into
humblefrom
mergify/bp/humble/pr-3239
Sep 4, 2026
Merged

Fix lock order inversion between GraphListener and Context (ABI compatible backport) (backport #3239)#3241
skyegalaxy merged 1 commit into
humblefrom
mergify/bp/humble/pr-3239

Conversation

@mergify

@mergify mergify Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Description

Avoids inverting lock order between GraphListener::start_if_not_started() and Context::shutdown() by registering the on_shutdown_callback before acquiring shutdown_mutex_. To avoid duplicate callback registration, a check has been added such that the shutdown callback is de-registered if this thread wasn't the one that started the graph listener (or, if shutting down, throw GraphListenerShutdownError.)

Tested against my jazzy container with my reproducing example from #2946. The diff ought to cleanly apply to kilted and humble as well.

Fixes #2946 / #3232 for humble / jazzy / kilted

Is this user-facing behavior change?

no

Did you use Generative AI?

Claude Fable 5

Additional Information

This is an automatic backport of pull request #3239 done by [Mergify](https://mergify.com).

@skyegalaxy Skyler Medeiros @mergify
register shutdown callback without shutdown mutex (#3239)
84b0d27 
Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com>
Co-authored-by: Skyler Medeiros <skye@polymathrobotics.com>
(cherry picked from commit efb669e)
@mergify mergify Bot mentioned this pull request Aug 28, 2026
Merged
@skyegalaxy

skyegalaxy commented Aug 28, 2026

Copy link
Copy Markdown
Member

Pulls: #3241
Gist: https://gist.githubusercontent.com/skyegalaxy/ba7d26dbc1305fd3a9665b4bff41a024/raw/6387fb25f5777ac6c5926e747932e5da1712bd09/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/20266

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

@skyegalaxy

skyegalaxy commented Aug 31, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Pulls: #3241
Gist: https://gist.githubusercontent.com/skyegalaxy/b91fd8a9f2d85dc531f2e7f55f461a62/raw/6387fb25f5777ac6c5926e747932e5da1712bd09/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/20286

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status (10171 -> 10174)
  • Windows Build Status

@skyegalaxy skyegalaxy self-assigned this Sep 3, 2026
@skyegalaxy

skyegalaxy commented Sep 3, 2026

Copy link
Copy Markdown
Member

@fujitatomoya @ahcorde - the rhel failures look like they're not related. shall I go ahead and merge?
image

@fujitatomoya

fujitatomoya commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

sounds good to me, but how about windows??? 🤔

@skyegalaxy

skyegalaxy commented Sep 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

huh, looks like it failed because rmw_fastrtps_dynamic_cpp couldn't be found?
I'll rerun CI anyway because that seems like perhaps a transient jenkins issue
image

@skyegalaxy

skyegalaxy commented Sep 4, 2026

Copy link
Copy Markdown
Member

Pulls: #3241
Gist: https://gist.githubusercontent.com/skyegalaxy/2dd7f18873750662f75a171cbdb301da/raw/6387fb25f5777ac6c5926e747932e5da1712bd09/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/20337

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

@jmachowinski

jmachowinski commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

we can't build humble on windows any more, this is good to go

@skyegalaxy skyegalaxy linked an issue Sep 4, 2026 that may be closed by this pull request
TSan lock-order-inversion reproducible via Nav2 test_bond #3232
Closed
@skyegalaxy
skyegalaxy merged commit 804a5e4 into humble Sep 4, 2026
2 checks passed
@ahcorde
ahcorde deleted the mergify/bp/humble/pr-3239 branch September 7, 2026 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fujitatomoya fujitatomoya fujitatomoya approved these changes

Assignees

@skyegalaxy skyegalaxy

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

TSan lock-order-inversion reproducible via Nav2 test_bond

3 participants

@skyegalaxy @fujitatomoya @jmachowinski