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 PublisherZMQ segfault upon destruction #440

Conversation

schornakj
Copy link
Contributor

Wait for any in-progress message sending to finish before deleting ZMQ. This fixes some cases where a segfault could occur when an instance of PublisherZMQ was destroyed.

I added unit tests based on the test case described in #426 (side note: that PR has a somewhat more complex solution to this same problem, but I pushed this one because I think the solution here is OK). I also added the bonus esoteric-yet-seen-in-the-wild case of deleting PublisherZMQ without even ticking the tree.

This is based on work by @JafarAbdi.

@Tradias
Copy link
Contributor

Tradias commented Oct 1, 2022

The reason I didn't go for this simple implementation is that the destructor will now block for up to min_time_between_msgs_, in my implementation it would complete "immediately".

@Tradias
Copy link
Contributor

Tradias commented Oct 1, 2022

Aside from the fact that PublisherZMQ::callback still has a race condition on send_pending_. Well not exactly on it, but it can potentially call std::async twice.

@facontidavide
Copy link
Collaborator

ok, thanks, I will reconsider and merge yours

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.

None yet

4 participants