Skip to content

Fix deadlock between wait_set condition_mutex and entity-specific mutexes (backport #992)#1001

Closed
mergify[bot] wants to merge 2 commits into
jazzyfrom
mergify/bp/jazzy/pr-992
Closed

Fix deadlock between wait_set condition_mutex and entity-specific mutexes (backport #992)#1001
mergify[bot] wants to merge 2 commits into
jazzyfrom
mergify/bp/jazzy/pr-992

Conversation

@mergify

@mergify mergify Bot commented Jun 18, 2026

Copy link
Copy Markdown

Description

Fix ABBA deadlock caused between the waitset's global condition_mutex and class-level internal mutexes in the RMW data structures. The fix follows the same pattern as what we have in EventsManger which was added as part of #937.

Added a test case to validate fix.

colcon test --packages-select test_rmw_zenoh_cpp

Fixes #921

Is this user-facing behavior change?

No

Did you use Generative AI?

Yes, Gemini 3

Additional Information


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

@mergify mergify Bot added the conflicts label Jun 18, 2026
@mergify

mergify Bot commented Jun 18, 2026

Copy link
Copy Markdown
Author

Cherry-pick of 5eaeb6d has failed:

On branch mergify/bp/jazzy/pr-992
Your branch is up to date with 'origin/jazzy'.

You are currently cherry-picking commit 5eaeb6d.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   rmw_zenoh_cpp/src/detail/guard_condition.cpp
	modified:   rmw_zenoh_cpp/src/detail/rmw_client_data.cpp
	modified:   rmw_zenoh_cpp/src/detail/rmw_service_data.cpp
	modified:   test_rmw_zenoh_cpp/CMakeLists.txt
	modified:   test_rmw_zenoh_cpp/package.xml
	new file:   test_rmw_zenoh_cpp/test/test_issue_921.cpp

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   rmw_zenoh_cpp/src/detail/rmw_subscription_data.cpp

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

…exes (#992)

Signed-off-by: Yadunund <yadunund@intrinsic.ai>
@Yadunund
Yadunund force-pushed the mergify/bp/jazzy/pr-992 branch from 4109bb1 to d13d477 Compare June 19, 2026 01:41
Comment thread test_rmw_zenoh_cpp/CMakeLists.txt Outdated
@mergify

mergify Bot commented Jun 19, 2026

Copy link
Copy Markdown
Author

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

  • Queue this pull request

@romainreignier

Copy link
Copy Markdown
Contributor

When using this version, every time I have tried to start a launch file with about 20 nodes, some nodes crashed on boot with this output:

Error: Timeout occurred (RCL_RET_TIMEOUT).

I have not dug enough to see if it is this exact commit that caused the issue.

@Yadunund

Copy link
Copy Markdown
Member

When using this version, every time I have tried to start a launch file with about 20 nodes, some nodes crashed on boot with this output:

Error: Timeout occurred (RCL_RET_TIMEOUT).

I have not dug enough to see if it is this exact commit that caused the issue.

@romainreignier I recommend opening another ticket with more details on the error and with a set of commands that will help us reproduce the issue to target a fix.

Signed-off-by: Yadunund <yadunund@intrinsic.ai>
@araitaiga

Copy link
Copy Markdown

@romainreignier

Error: Timeout occurred (RCL_RET_TIMEOUT).

I've confirmed a similar crash on both rolling and this branch. I've opened an issue (#1003 ) for rolling with a minimal reproduction. Could you check whether it matches your case?

@romainreignier

Copy link
Copy Markdown
Contributor

@araitaiga Thanks a lot for opening the issue, I could not find a time slot this week to do further testing on this.

@jmachowinski

Copy link
Copy Markdown
Contributor

I think this patch is broken as it introduces a use after free. Closing this for now.

@jmachowinski

Copy link
Copy Markdown
Contributor

@romainreignier @araitaiga There is a new fix at #1005
It would be great if you could test the patch and report the result in the pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants