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 callback accessing invalid reference to promise #268

Conversation

achim-k
Copy link
Collaborator

@achim-k achim-k commented Oct 23, 2023

Public-Facing Changes

Fix callback accessing invalid reference to promise

Description

For ROS1, the service request and response types can only be retrieved by opening a connection to the service server. For this, we use ros::ServiceManager::instance()->createServiceServerLink which under the hood keeps a copy of the returned pointer, effectively keeping the connection alive. This patch makes sure that we drop the connection when we either have received the header or when the timeout has been exceeded. This avoids that the header-received callback is being called with an invalid reference to the promise.

Fixes #267
Resolves FG-5366

Copy link
Member

@jtbandes jtbandes left a comment

Choose a reason for hiding this comment

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

Should we try this removeServiceServerLink method? (I don't really see any docs on how these should be used, just googling around)

@achim-k
Copy link
Collaborator Author

achim-k commented Oct 23, 2023

Should we try this removeServiceServerLink method? (I don't really see any docs on how these should be used, just googling around)

I have tried this originally, but I only got lock error exceptions when doing so. I don't think that this API is really meant to be used other than internally.

Copy link
Member

@jtbandes jtbandes left a comment

Choose a reason for hiding this comment

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

👍

@achim-k achim-k merged commit e52bb9d into main Oct 23, 2023
11 checks passed
@achim-k achim-k deleted the achim/fg-5366-crash-in-foxglove-ros1-due-to-pointer-foxgloveros-foxglove branch October 23, 2023 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

crash in foxglove ros1 due to pointer
2 participants