Fix segmentation fault on exit for macOS tests

[srmainwaring]

🦟 Bug fix

Fixes #442

Summary

See #442 for details on how to reproduce.

Add a link dependency to dartsim_plugin in the failing test(s). The dependency is present in the tests contained in gz-physics/dart but not for those in gz-physics/src.

This prevent the segmentation fault on exit, but does not really address the root cause of why the SingletonFactory for the CollisionDetector has a segmentation faulting without the explicit link.

As mentioned in the comment, the error occurs in the destruction of the static Registrar variable for the DARTCollisionDetector. The std::function associated with the lambda in the registration entry below is invalid at the point the Factory destructor is called.

DARTCollisionDetector::Registrar<DARTCollisionDetector>
    DARTCollisionDetector::mRegistrar{
        DARTCollisionDetector::getStaticType(),
        []() -> std::shared_ptr<dart::collision::DARTCollisionDetector> {
          return dart::collision::DARTCollisionDetector::create();
        }};

It's possible the explicit link prevents the shared library from being unloaded and so the process exits cleanly (the same result occurs if the plugin LoadLib call is made with nodelete=true).

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.

[codecov]

Codecov Report

Merging #510 (97808ad) into gz-physics6 (63a7486) will not change coverage.
The diff coverage is n/a.

❗ Current head 97808ad differs from pull request most recent head 70951e2. Consider uploading reports for the commit 70951e2 to get more accurate results

@@             Coverage Diff              @@
##           gz-physics6     #510   +/-   ##
============================================
  Coverage        75.71%   75.71%           
============================================
  Files              142      142           
  Lines             7199     7199           
============================================
  Hits              5451     5451           
  Misses            1748     1748           

[azeey]

Looks like CI runs on Bigsur macOS machines, but we don't have a PR CI job for Monterey. Is there a way to test this on Monterey? /cc @Crola1702

[Crola1702]

Looks like CI runs on Bigsur macOS machines, but we don't have a PR CI job for Monterey. Is there a way to test this on Monterey?

ignition_physics-ci-pr_any-homebrew-amd64 job runs on osx machines. It's randomly assigned to bigsur or osx_monterey.

AFAIK The only way to test in on monterey right now is:

1. Run multiple builds on this job and check only the one that ran on monterey
2. Put bigsur machines offline, run the build (it will be assigned to monterey machine), and put bigsur machines online again.

WDYT? @j-rivero

[scpeters]

I've tested it locally on my macOS Monterey laptop, and this fixes UNIT_FindFeatures_TEST but not the COMMON_TEST_* failures listed in #442 (comment)

I'd rather not link the common tests against dartsim, so perhaps we can add a helper function to call LoadLib(libNameMatchingDart, true) and LoadLib(allOtherLibs, false)?

[srmainwaring]

I'd rather not link the common tests against dartsim, so perhaps we can add a helper function to call LoadLib(libNameMatchingDart, true) and LoadLib(allOtherLibs, false)?

Thought that might be the case, and I think both approaches prevent the plugin from unloading.

I'm not able to reproduce the seg faults on the COMMON_TEST_* listed in the issue. This on an Intel MacPro, Monterey 12.6.2 with Xcode 14.2 on a from source build. All these tests run clean in my environment (without this change). I haven't updated brew on this machine for a while, so that will be the next thing.

[scpeters]

I'm not able to reproduce the seg faults on the COMMON_TEST_* listed in the issue. This on an Intel MacPro, Monterey 12.6.2 with Xcode 14.2 on a from source build. All these tests run clean in my environment (without this change). I haven't updated brew on this machine for a while, so that will be the next thing.

Interesting; can you check which version of dartsim you currently have installed? I have 6.13.0, which is the current version in homebrew-core

[srmainwaring]

which version of dartsim you currently have installed?

I'm using dartsim@6.10.0 (following the install docs https://gazebosim.org/docs/garden/install_osx_src). I get the same result using the osrf binary build from brew and a source build from the osrf fork.

[scpeters]

which version of dartsim you currently have installed?

I'm using dartsim@6.10.0 (following the install docs https://gazebosim.org/docs/garden/install_osx_src). I get the same result using the osrf binary build from brew and a source build from the osrf fork.

The from-source installation instructions may be out of date, since fortress and garden both use the homebrew-core version of dartsim:

• https://github.com/osrf/homebrew-simulation/blob/master/Formula/ignition-physics5.rb#L19
• https://github.com/osrf/homebrew-simulation/blob/master/Formula/gz-physics6.rb#L17

Can you try with homebrew/core/dartsim instead of osrf/simulation/dartsim@6.10.0? I'll do the reverse and see what happens if I use our fork of version 6.10.0

[srmainwaring]

Can you try with homebrew/core/dartsim

Using brew dartsim (/usr/local/Cellar/dartsim/6.13.0_3) the COMMON_TEST_* tests segmentation fault on exit.

[azeey]

As another data point, I had to remove libdart-collision-bullet from INTEGRATION_DoublePendulum to fix a segfault in #504. I wonder if that would work here.

[azeey]

Alternative fix #529

[azeey]

Closing since #529 was merged.