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

Fix queue blocking #464

Merged
merged 6 commits into from
Apr 7, 2020
Merged

Fix queue blocking #464

merged 6 commits into from
Apr 7, 2020

Conversation

mvollrath
Copy link
Contributor

QueueMessageHandler.run was holding onto its lock while writing out to the server. Since the websocket server can block outgoing messages now, this was causing cross-client blocking by jamming up the rospy Subscriber thread.

By releasing the lock before writing, we let this handler work the way it was always intended.

The handler's internal queue was switched to a deque for simplicity.

The thread was holding the lock while pushing to a potentially blocking
function.  Rewrite the logic and use a deque while we're at it.
Unit tests were sending a message of 0 which was tripping up this logic.

Doesn't hurt to be explicit.
@reinzor
Copy link

reinzor commented Mar 10, 2020

We will test with this the coming weeks and see if the hanging still occurs.

@reinzor
Copy link

reinzor commented Mar 13, 2020

Still seems to occur, some topics are not coming in anymore. Latched topics keep old state and are not updating.

@mvollrath
Copy link
Contributor Author

@jihoonl @viktorku any questions?

@mvollrath
Copy link
Contributor Author

@jihoonl I've added a test which fails on develop to illustrate what this is doing.

@jihoonl
Copy link
Member

jihoonl commented Apr 7, 2020

Sorry for late response. Looks good. I have full trust on you. thank @mvollrath!

@mvollrath mvollrath merged commit 22e3652 into develop Apr 7, 2020
@jtbandes jtbandes deleted the fix_queue_blocking branch August 13, 2021 22:18
DomenicP added a commit to GeckoRobotics/rosbridge_suite that referenced this pull request Sep 30, 2021
DomenicP added a commit to GeckoRobotics/rosbridge_suite that referenced this pull request Sep 30, 2021
DomenicP added a commit to GeckoRobotics/rosbridge_suite that referenced this pull request Sep 30, 2021
DomenicP added a commit to GeckoRobotics/rosbridge_suite that referenced this pull request Oct 1, 2021
DomenicP added a commit to GeckoRobotics/rosbridge_suite that referenced this pull request Oct 1, 2021
amacneil pushed a commit that referenced this pull request Oct 7, 2021
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

Successfully merging this pull request may close these issues.

3 participants