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

[19743] Add expiration checks in database fragile pointers #258

Merged
merged 1 commit into from
Dec 16, 2024

Conversation

Carlosespicur
Copy link
Contributor

@Carlosespicur Carlosespicur commented Dec 3, 2024

Main changes

This PR fixes a crash reported in Fast DDS Monitor that can be reproduced by following the steps below:

  1. Run an endpoint and open Fast DDS Monitor
  2. Select a locator of the previous endpoint
  3. Delete the endpoint or locator (it will become inactive in the monitor)
  4. Delete inactive entities

In the database, associated locator entities are stored as fragile references (fragile_ptr) but returned as shared references (shared_ptr). When an associated locator entity (in this case a subscriber or endpoint) is deleted, its associated fragile_ptr becomes expired and is returned as nullptr when casted to shared_ptr, causing memory errors when trying to access an object member.

There are two ways to deal with this problem:

  1. Check whether fragile_ptr is expired or not before casting to shared_ptr.
  2. Avoid using shared pointers and use fragile_ptrs instead. fragile_ptr is designed to avoid these memory errors without breaking the API.

@Carlosespicur Carlosespicur changed the base branch from main to feature/represent-qos-incompatibility December 3, 2024 16:03
depink5
depink5 previously approved these changes Dec 4, 2024
@Carlosespicur Carlosespicur force-pushed the feature/represent-qos-incompatibility branch from b2fd857 to a651aab Compare December 5, 2024 07:43
Base automatically changed from feature/represent-qos-incompatibility to main December 10, 2024 08:02
@rsanchez15 rsanchez15 dismissed depink5’s stale review December 10, 2024 08:02

The base branch was changed.

@Carlosespicur Carlosespicur force-pushed the hotfix/refresh-inactive-entities branch from 211507e to 4110d61 Compare December 12, 2024 08:19
@Carlosespicur Carlosespicur force-pushed the hotfix/refresh-inactive-entities branch from 4110d61 to 82751b0 Compare December 12, 2024 08:22
@rsanchez15 rsanchez15 force-pushed the hotfix/refresh-inactive-entities branch from 82751b0 to e6f1675 Compare December 13, 2024 06:44
@rsanchez15 rsanchez15 merged commit 8129d05 into main Dec 16, 2024
26 checks passed
@rsanchez15 rsanchez15 deleted the hotfix/refresh-inactive-entities branch December 16, 2024 07:02
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