`reserveCells` crashes `mint` mesh inside Umpire #1271

gunney1 · 2024-02-06T22:21:03Z

Crash reserving space for `UnstructuredMesh`

This PR is a bug fix
It does the following:
- Adds tests that verify the crash (which happens on our docker platforms)
- Fix the bug.

The cause of the bug is the combination of static Umpire library with shared Axom library in Docker. Somehow, this resulted in 2 singleton ResourceManagers. The root cause of the crash in reserveCells was when the second singleton tried to reallocate memory allocated by the first. Fixed by building Umpire shared. We updated the Docker image to make Umpire shared.

This crash had been plaguing PR #1263. The MarchingCubes test crashed while asking mint::UnstructuredMesh to reserve space.

Two identical tests were added to monitor this behavior:

In src/axom/quest/tests/quest_initialize.cpp
In src/axom/mint/tests/mint_mesh.cpp

Interesting behaviors:

The test added to quest reproduced the crash, but the one added to mint did not.
The crashes only occurred in our docker builds.

Addresses issue #1287

gunney1 · 2024-02-22T00:07:23Z

Some new observations:

Based on @white238's guess, I disabled MPI (and MFEM, which was configured with MPI) from the failing config. This fixed the failure. Not exactly sure what the mechanism of failure was.
I tried to construct the UnstructuredMesh without using Sidre. This also gets past the crash. This provides a work-around for this bug, but it effectively disables some Sidre-dependent features when using the UnstructuredMesh.

…iate-reserve

gunney1 · 2024-06-10T23:28:10Z

After merging in the latest develop branch, the failed test (quest_initialize) continues to fail. But if I remove the -DBUILD_SHARED_LIBS=ON from the configuration, the tests pass. @white238 What do you think of this clue?

…iate-reserve

I don't know the true cause of the crash or why it only exhibits on our docker environments. An ideal fix based on an understanding of the bug should be pursued. This commit also backs out work-arounds for avoiding trigging the crash.

New tests reproduces the crash without involving mint and seems to implicate sidre::View memory management.

source of error.

…iate-reserve

See if that fixes the crash. Our hypothesis is that static Umpire and shared Axom somehow leads to two ResourceManager singletons.

…iate-reserve

rhornung67 · 2024-10-10T17:09:03Z

azure-pipelines.yml

@@ -9,8 +9,8 @@ variables:
  DO_BUILD: 'yes'
  DO_TEST: 'yes'
  DO_CLEAN: 'no'
-  CLANG14_IMAGENAME: 'axom/tpls:clang-14_09-23-24_17h-07m'
-  GCC13_IMAGENAME: 'axom/tpls:gcc-13_09-23-24_17h-07m'
+  CLANG14_IMAGENAME: 'axom/tpls:clang-14_10-10-24_01h-31m'


Is this change required?

Yes. The new images have Umpire shared libs. That's the key to the bug fix.

@white238 @kennyweiss are you OK with switching to Umpire shared libs for azure testing?

We (@kennyweiss , @gunney1 , @davidbeckingsale ) met about this last night. Basically a static umpire with a shared axom creates multiple singletons in Umpire. We need to not allow this to happen in our Spack recipe as well but this seems to be the only way around this.

Do you mean add a constraint in the Axom Spack package that static Umpire conflicts with shared Axom?

yes and/or a conflict

Yes -- the three of us had a meeting yesterday w/ @davidbeckingsale and concluded that the static umpire combined w/ a shared axom in the docker configuration was leading to more than one umpire instance.

There might be a solution to this w/ improved linking orders and/or linker flags, but it will be difficult to track down in the short term, and is not necessary for @gunney1's actual deliverable (i.e. the CI is testing a different configuration than the one we actually need). @gunney1 is planning to create an issue summarizing that discussion so we can look into it in the future.

It's also worth noting that we've encountered similar issues in the past when mixing libraries w/ singletons with upstream shared or Fortran libraries (e.g. #865 and #270).

Edit: My response took too long and @white238 beat me to the punch

kennyweiss · 2024-10-10T18:24:27Z

scripts/spack/configs/docker/ubuntu22/spack.yaml

@@ -165,7 +165,7 @@ spack:
    scr:
      require: "@3.0.1~shared~tests~examples"
    umpire:
-      require: "@2024.07.0~shared~examples~werror"
+      require: "@2024.07.0+shared~examples~werror"


@rhornung67 -- the change to the docker TPL configuration is actually in this line -- we are now forcing Umpire to build as a shared library for better compatibility w/ the shared axom config.

kennyweiss

Thanks @gunney1

Please also test this branch w/ AXOM_MINT_USE_SIDRE=OFF to ensure that all sidre usage w/ mint is properly guarded (see: #1435).

src/axom/quest/tests/quest_initialize.cpp

… mint.

gunney1 self-assigned this Feb 6, 2024

gunney1 added Sidre Issues related to Axom's 'sidre' component Quest Issues related to Axom's 'quest' component Mint Issues related to Axom's 'mint' component labels Feb 6, 2024

Add reproducers for bug crashing UnstructuredMesh::reserveCells().

f25bdc4

gunney1 force-pushed the bugfix/gunney/ug-immediate-reserve branch from 643c441 to f25bdc4 Compare February 7, 2024 00:36

Add missing include guard.

ba90110

gunney1 added the bug Something isn't working label Feb 7, 2024

gunney1 added this to the April 2024 Release milestone Feb 7, 2024

gunney1 added a commit that referenced this pull request Feb 22, 2024

Temporary work-around for un-related bug being addressed by PR #1271.

779de44

gunney1 mentioned this pull request Mar 11, 2024

Crash when UnstructuredMesh allocates memory with Umpire in docker #1287

Open

gunney1 added 2 commits March 26, 2024 12:02

Merge remote-tracking branch 'gh/develop' into bugfix/gunney/ug-immed…

10972e8

…iate-reserve

Merge remote-tracking branch 'gh/develop' into bugfix/gunney/ug-immed…

0e0d51a

…iate-reserve

gunney1 added 2 commits October 4, 2024 16:50

Merge remote-tracking branch 'gh/develop' into bugfix/gunney/ug-immed…

2076165

…iate-reserve

gunney1 force-pushed the bugfix/gunney/ug-immediate-reserve branch from 0c089b5 to c96baed Compare October 6, 2024 13:49

gunney1 added 12 commits October 9, 2024 03:48

Disable the work-around and add tests that bypass mint.

c36ec8c

New tests reproduces the crash without involving mint and seems to implicate sidre::View memory management.

Fix an un-related compile fail in the new test code.

cdf6ffb

Add test to allocate and reallocate in sidre package.

681d035

Add test that use only Buffer interface, eliminating Views code as

276c37d

source of error.

Add tests to verify allocate/deallocate works without sidre::Buffer.

cc83e71

Merge remote-tracking branch 'gh/develop' into bugfix/gunney/ug-immed…

49abe98

…iate-reserve

Fix a bug in new test.

b8cf67d

Build Umpire TPL shared in docker.

ccced65

See if that fixes the crash. Our hypothesis is that static Umpire and shared Axom somehow leads to two ResourceManager singletons.

Rebuild docker images with shared Umpire library.

7a449f9

Use new docker images with shared Umpire libs.

5202846

Backout work-around and remove a redundant test.

3aae97a

Merge remote-tracking branch 'gh/develop' into bugfix/gunney/ug-immed…

7c2bcc2

…iate-reserve

gunney1 marked this pull request as ready for review October 10, 2024 17:04

gunney1 requested review from davidbeckingsale, white238, BradWhitlock, kennyweiss, rhornung67 and agcapps October 10, 2024 17:04

rhornung67 reviewed Oct 10, 2024

View reviewed changes

rhornung67 approved these changes Oct 10, 2024

View reviewed changes

kennyweiss reviewed Oct 10, 2024

View reviewed changes

kennyweiss approved these changes Oct 10, 2024

View reviewed changes

src/axom/quest/tests/quest_initialize.cpp Show resolved Hide resolved

src/axom/quest/tests/quest_initialize.cpp Show resolved Hide resolved

src/axom/quest/tests/quest_initialize.cpp Show resolved Hide resolved

gunney1 added 3 commits October 10, 2024 12:58

Guard code for non-Sidre builds.

6c50977

Add comment explaining why new tests are in quest instead of sidre or…

cf9f910

… mint.

Autoformat.

e9844ea

gunney1 merged commit f596660 into develop Oct 10, 2024
13 checks passed

gunney1 deleted the bugfix/gunney/ug-immediate-reserve branch October 10, 2024 21:54

gunney1 mentioned this pull request Oct 11, 2024

Multiple Umpire instances caused by shared library #1444

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`reserveCells` crashes `mint` mesh inside Umpire #1271

`reserveCells` crashes `mint` mesh inside Umpire #1271

gunney1 commented Feb 6, 2024 •

edited

Loading

gunney1 commented Feb 22, 2024

gunney1 commented Jun 10, 2024

rhornung67 Oct 10, 2024

gunney1 Oct 10, 2024 •

edited

Loading

rhornung67 Oct 10, 2024

white238 Oct 10, 2024

rhornung67 Oct 10, 2024

white238 Oct 10, 2024

kennyweiss Oct 10, 2024 •

edited

Loading

kennyweiss Oct 10, 2024

kennyweiss left a comment

reserveCells crashes mint mesh inside Umpire #1271

reserveCells crashes mint mesh inside Umpire #1271

Conversation

gunney1 commented Feb 6, 2024 • edited Loading

Crash reserving space for UnstructuredMesh

gunney1 commented Feb 22, 2024

gunney1 commented Jun 10, 2024

rhornung67 Oct 10, 2024

Choose a reason for hiding this comment

gunney1 Oct 10, 2024 • edited Loading

Choose a reason for hiding this comment

rhornung67 Oct 10, 2024

Choose a reason for hiding this comment

white238 Oct 10, 2024

Choose a reason for hiding this comment

rhornung67 Oct 10, 2024

Choose a reason for hiding this comment

white238 Oct 10, 2024

Choose a reason for hiding this comment

kennyweiss Oct 10, 2024 • edited Loading

Choose a reason for hiding this comment

kennyweiss Oct 10, 2024

Choose a reason for hiding this comment

kennyweiss left a comment

Choose a reason for hiding this comment

`reserveCells` crashes `mint` mesh inside Umpire #1271

`reserveCells` crashes `mint` mesh inside Umpire #1271

gunney1 commented Feb 6, 2024 •

edited

Loading

Crash reserving space for `UnstructuredMesh`

gunney1 Oct 10, 2024 •

edited

Loading

kennyweiss Oct 10, 2024 •

edited

Loading