Skip to content
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

Lack of Thread Synchronization #390

Open
git-afsantos opened this issue Apr 13, 2018 · 2 comments
Open

Lack of Thread Synchronization #390

git-afsantos opened this issue Apr 13, 2018 · 2 comments
Assignees

Comments

@git-afsantos
Copy link

Kobuki's nodelets seem to lack proper thread synchronization mechanisms (e.g. mutex), to protect against the reading of inconsistent values (mid-write) and stale data (writes made by one thread are not visible to another). This is true for kobuki_safety_controller, kobuki_random_walker and kobuki_node (this one to a lesser extent).

Are these mechanisms provided somewhere else (e.g. super-classes), and I just missed them? Otherwise, these nodelets are relying on chance to behave correctly.

Example (with kobuki_safety_controller)

After the robot bumps into an obstacle, the safety controller receives a bumper event and SafetyController::bumperEventCB is called on the "main" thread of the nodelet. This callback is supposed to update, say, bumper_center_pressed_ = true;.

The update thread will, eventually, call spin, where it attempts to read bumper_center_pressed_ . Since there are no synchronization mechanisms of any sort, there is no guarantee that this thread will ever see true on this variable (if it relies on the processor's cache). This will result in the safety action not being triggered, leaving the robot to keep pushing the obstacle.

@clalancette clalancette self-assigned this Jan 10, 2020
@clalancette
Copy link
Collaborator

Absolutely, this is a problem I've noticed as well. I'll assign this to myself to work on (though my work will be delayed a bit).

@stonier
Copy link
Member

stonier commented Jan 10, 2020

Aye - I concur. I'll also keep an eye out when moving on the ROS2 upgrade.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants