Skip to content

Fix realtime publisher race condition upon initialization #309

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

saikishor
Copy link
Member

Should fix : #24
Closes: #308

@bijoua29 can you test this and see if this helps?

Copy link

@bijoua29 bijoua29 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note my comments and see if you agree

@bijoua29
Copy link

bijoua29 commented Apr 5, 2025

@saikishor Yes, I will test it today or tomorrow. Thanks for fixing this.

Copy link
Contributor

@christophfroehlich christophfroehlich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great.

So we don't need something like

while (!publisher_->is_running()) {
    std::this_thread::sleep_for(std::chrono::milliseconds(10));
}

in the application code, right?

@saikishor
Copy link
Member Author

This looks great.

So we don't need something like

while (!publisher_->is_running()) {
    std::this_thread::sleep_for(std::chrono::milliseconds(10));
}

in the application code, right?

Yess! Exactly

@bijoua29
Copy link

bijoua29 commented Apr 6, 2025

@saikishor I ran a test 20 times pre-PR and had a 85% failure rate. I ran another test 20 times post-PR and had 100% success. So I think the PR fixes the problem. I am approving the PR.

Copy link

@bijoua29 bijoua29 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@christophfroehlich christophfroehlich added backport-humble This label should be used by maintainers only! Label triggers PR backport to ROS 2 humble. backport-jazzy This label should be used by maintainers only! Label triggers PR backport to ROS 2 humble. labels Apr 6, 2025
@christophfroehlich christophfroehlich merged commit 0d605d2 into ros-controls:master Apr 6, 2025
8 of 12 checks passed
mergify bot pushed a commit that referenced this pull request Apr 6, 2025
mergify bot pushed a commit that referenced this pull request Apr 6, 2025
christophfroehlich pushed a commit that referenced this pull request Apr 6, 2025
christophfroehlich pushed a commit that referenced this pull request Apr 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-humble This label should be used by maintainers only! Label triggers PR backport to ROS 2 humble. backport-jazzy This label should be used by maintainers only! Label triggers PR backport to ROS 2 humble.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Latched RealtimePublisher is racy on startup
3 participants