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

Unregister collision detectors when the darstim plugin is unloaded #529

Merged
merged 2 commits into from
Aug 22, 2023

Conversation

azeey
Copy link
Contributor

@azeey azeey commented Aug 10, 2023

🦟 Bug fix

Partial fix for #442

Summary

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
@github-actions github-actions bot added the 🌱 garden Ignition Garden label Aug 10, 2023
@azeey
Copy link
Contributor Author

azeey commented Aug 10, 2023

Monterey build: Build Status

@codecov
Copy link

codecov bot commented Aug 10, 2023

Codecov Report

Merging #529 (0b824ab) into gz-physics6 (871bab7) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 0b824ab differs from pull request most recent head b10917a. Consider uploading reports for the commit b10917a to get more accurate results

@@             Coverage Diff              @@
##           gz-physics6     #529   +/-   ##
============================================
  Coverage        76.18%   76.19%           
============================================
  Files              142      142           
  Lines             7251     7254    +3     
============================================
+ Hits              5524     5527    +3     
  Misses            1727     1727           
Files Changed Coverage Δ
dartsim/src/plugin.cc 100.00% <100.00%> (ø)

@scpeters
Copy link
Member

Monterey build: Build Status

there is still one failing test in this build, but it did fix the UNIT_FindFeatures and INTEGRATION_DoublePendulum test failures, so this is definitely an improvement

@scpeters
Copy link
Member

Monterey build: Build Status

there is still one failing test in this build, but it did fix the UNIT_FindFeatures and INTEGRATION_DoublePendulum test failures, so this is definitely an improvement

the COMMON_TEST_joint_features_bullet-featherstone test is now seg-faulting in JointFeaturesAttachDetachTest/0.JointAttachDetachSpawnedModel. I collected a backtrace implicating the btCollisionWorld destructor with EXEC_BAD_ACCESS. I will post the backtrace in #442.

@azeey
Copy link
Contributor Author

azeey commented Aug 22, 2023

Yes, the segfaults in bullet-featherstone are a separate problem. This is not meant to fix that. I'll update the description to say it partially fixes #442.

BTW, I believe the segfaults in bullet-featherstone are due to the order of destruction of unique_ptrs in Base.hh. Bullet seems to have a lot of interdependencies, so deleting objects in the right order is tricky. I have been able to reproduce the problem on linux when running under Valgrind, but I haven't had the chance to fix the issue.

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
@azeey azeey marked this pull request as ready for review August 22, 2023 15:15
@azeey azeey merged commit 5343ab9 into gazebosim:gz-physics6 Aug 22, 2023
4 checks passed
@azeey azeey deleted the fix_segfault_monterey branch August 22, 2023 19:42
scpeters pushed a commit that referenced this pull request Sep 1, 2023
)

Fixes a segfault that occurs due to destructors being removed from memory before they're called.
---------

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
scpeters pushed a commit that referenced this pull request Sep 5, 2023
)

Fixes a segfault that occurs due to destructors being removed from memory before they're called.
---------

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
scpeters pushed a commit that referenced this pull request Oct 30, 2023
)

Fixes a segfault that occurs due to destructors being removed from memory before they're called.
---------

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌱 garden Ignition Garden
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants