fix(posegraph): Resolve segfault in loop closure visualization#78
Open
qza36 wants to merge 1 commit intoMIT-SPARK:mainfrom
Open
fix(posegraph): Resolve segfault in loop closure visualization#78qza36 wants to merge 1 commit intoMIT-SPARK:mainfrom
qza36 wants to merge 1 commit intoMIT-SPARK:mainfrom
Conversation
A race condition between the 20Hz visualization timer and the registration thread caused a segmentation fault when a loop closure attempt failed. The root cause was that the `need_lc_cloud_vis_update_` flag was set to `true` immediately after performing registration, but before checking if the registration was valid. The asynchronous visualization timer would then read this flag, attempt to access data from the failed registration (which contained null pointers for clouds like `FinalAlignedCloud`), and crash. This commit resolves the issue through a two-pronged approach: 1. **Corrected Logic:** The `need_lc_cloud_vis_update_` flag is now only set to `true` inside the `if (reg_output.is_valid_)` block, ensuring that the visualization is only triggered for successful loop closures. 2. **Defensive Programming:** Added null pointer checks within the `visualizeLoopClosureClouds` function itself. This makes the visualizer more robust and prevents crashes even if similar logic errors occur in the future.
Member
|
Hmmm, interesting, but my intention is to show them regardless of whether the registration fails. Could you elaborate on that? Q1. Which data did you use?
Where's this part? |
Author
Member
|
Oh, what I'm saying is, you don't seem to push the screenshot things. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

A race condition between the 20Hz visualization timer and the registration thread caused a segmentation fault when a loop closure attempt failed.
The root cause was that the
need_lc_cloud_vis_update_flag was set totrueimmediately after performing registration, but before checking if the registration was valid. The asynchronous visualization timer would then read this flag, attempt to access data from the failed registration (which contained null pointers for clouds likeFinalAlignedCloud), and crash.This commit resolves the issue through a two-pronged approach:
Corrected Logic: The
need_lc_cloud_vis_update_flag is now only set totrueinside theif (reg_output.is_valid_)block, ensuring that the visualization is only triggered for successful loop closures.Defensive Programming: Added null pointer checks within the
visualizeLoopClosureCloudsfunction itself. This makes the visualizer more robust and prevents crashes even if similar logic errors occur in the future.