Fix a deadlock between endpoint creation and discovery queries - #440
Open
johannesschrimpf wants to merge 1 commit into
Open
Fix a deadlock between endpoint creation and discovery queries#440johannesschrimpf wants to merge 1 commit into
johannesschrimpf wants to merge 1 commit into
Conversation
Concurrent endpoint creation and discovery snapshot queries on the same DomainParticipant could deadlock. Writer and reader creation held the DiscoveryDB write lock while building records that acquired the participant mutex (dpi). The discovered_* accessors acquired dpi first and then read DiscoveryDB, producing an ABBA lock cycle. Build writer and reader discovery records before taking the database write lock. Split local reader record construction from insertion so participant access happens outside the database critical section. In the three snapshot accessors, clone the database handle under dpi and release the participant mutex before reading the database. Also release the writer's database guard before the blocking send to add_writer_sender. That bounded channel is drained by the participant event loop, which itself reads DiscoveryDB; holding the write guard while waiting for channel capacity could prevent the consumer from making progress. Document these non-overlapping lock scopes and identify find_topic() as the remaining path that holds dpi while accessing DiscoveryDB. find_topic() continues to hold the participant mutex during its lookup and can delay other participant calls until it returns. Add a regression test that races endpoint creation against discovery snapshots in a child process. Complete publisher/subscriber setup before spawning barrier workers, exercise every accessor at least once, and fail after a 30-second deadline instead of hanging the test harness. Require a dedicated child completion exit code so an unmatched test filter cannot pass silently. Report child failures separately from a possible deadlock; the exact interleavings remain scheduler-dependent. A separate sanity test checks that a local writer's topic is visible. Reuse discovery_db_read for the snapshot accessors' poison handling. Validation on macOS with separate unfixed/fixed Cargo build directories: - The updated race test reaches the 30-second deadlock deadline on the unchanged pre-fix base. - Both focused integration tests pass with default features and security. - Temporary unmatched-filter and creator-setup-panic probes fail promptly through the child-failure path, rather than passing or timing out. - The integration test passes nightly-2026-09-08 rustfmt; git diff whitespace checks pass. Existing crate-wide formatting failures are outside this lock-order fix. Assisted-by: Claude Fable 5.1 Assisted-by: OpenAI Codex (gpt-6-astra)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I ran into this deadlock while writing a ROS node using ros2-client.
Creating readers or writers while another thread queries discovery information could deadlock. Endpoint creation took the DiscoveryDB lock and then the participant mutex, while discovery queries took those locks in the opposite order.
This change builds the endpoint discovery records before locking the database. Discovery queries now briefly take the participant mutex to obtain the database handle, release it, and then read the database.
The snapshot-reading code moves into the public DomainParticipant methods so the participant mutex can be released before acquiring the database read lock. The now-unused methods in DomainParticipantDisc and DomainParticipantInner are removed. The externally accessible API is unchanged.
The writer path also releases the database lock before sending to the participant event loop. That send can block when its channel is full, and the event loop may itself need the database lock to make progress.
I added a regression test that creates endpoints while querying discovery on the same participant:
Developed with assistance from Claude Fable 5.1 and OpenAI Codex (gpt-6-astra).