Skip to content
This repository was archived by the owner on May 31, 2025. It is now read-only.

Fix race condition that lead to miss first message#1054

Closed
kamibo wants to merge 3 commits intoros:indigo-develfrom
kamibo:indigo-devel
Closed

Fix race condition that lead to miss first message#1054
kamibo wants to merge 3 commits intoros:indigo-develfrom
kamibo:indigo-devel

Conversation

@kamibo
Copy link
Copy Markdown

@kamibo kamibo commented May 11, 2017

Callback queue waits for callback from "callOne" method.
When internal queue is not empty this last method succeeded even if id
info mapping does not contains related callback's id.
In this case, first callback (for one id) is never called since
"addCallback" method first push callback into internal queue and then
set info mapping.

So id info mapping has to be set before push callback into interal
queue. Otherwise first message could be ignored.

Attached file highlights this issue.
ros_callback_queue_bug.cpp.zip

Camille Bordignon and others added 2 commits May 11, 2017 17:27
Callback queue waits for callback from "callOne" method.
When internal queue is not empty this last method succeeded even if id
info mapping does not contains related callback's id.
In this case, first callback (for one id) is never called since
"addCallback" method first push callback into internal queue and *then*
set info mapping.

So id info mapping has to be set before push callback into internal
queue. Otherwise first message could be ignored.
@esteve
Copy link
Copy Markdown
Member

esteve commented May 17, 2017

@kamibo I've pushed kamibo#1 which includes your fix and a test for the race condition. I've removed the C++11 and C++14 parts from the test and replaced them with Boost so it builds fine on a strict Indigo.

@dirk-thomas
Copy link
Copy Markdown
Member

@kamibo Can you please incorporate the test provided by @esteve and target the latest development branch with this PR (lunar-devel).

@esteve
Copy link
Copy Markdown
Member

esteve commented May 18, 2017

@kamibo I've created #1058 which is just a rebase of this branch on top of lunar-devel

@dirk-thomas do you need a PR for Kinetic as well or you'll back/forward port it yourself?

@kamibo
Copy link
Copy Markdown
Author

kamibo commented May 18, 2017

Thanks @esteve it's perfect !

Added test for addCallback race condition
@dirk-thomas
Copy link
Copy Markdown
Member

Closing this in favor of #1058. The patch will be considered for backporting into older distros from the lunar-devel branch.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants