OpenVSP 3.50.4 VSPAERO crashes with default wing

[Iordanis Sapidis]

I am running Mint OS 22.2 and when running VSPAERO OpenVSP crashes with random errors/segfaults. One of the errors is the following:

void PGEdge::ReplaceNode(const PGNode*, PGNode*): Assertion `0' failed.

Stack trace:

vsp: /home/iordanis/Desktop/OpenVSP_debug/OpenVSP/src/geom_core/PGMesh.cpp:639: void PGEdge::ReplaceNode(const PGNode*, PGNode*): Assertion `0' failed.
==32291==
==32291== Process terminating with default action of signal 6 (SIGABRT)
==32291==    at 0x5015B2C: __pthread_kill_implementation (pthread_kill.c:44)
==32291==    by 0x5015B2C: __pthread_kill_internal (pthread_kill.c:78)
==32291==    by 0x5015B2C: pthread_kill@@GLIBC_2.34 (pthread_kill.c:89)
==32291==    by 0x4FBC27D: raise (raise.c:26)
==32291==    by 0x4F9F8FE: abort (abort.c:79)
==32291==    by 0x4F9F81A: __assert_fail_base.cold (assert.c:96)
==32291==    by 0x4FB2516: __assert_fail (assert.c:105)
==32291==    by 0x82C2C3: PGEdge::ReplaceNode(PGNode const*, PGNode*) (PGMesh.cpp:639)
==32291==    by 0x82AA65: PGNode::EdgeForgetNode(PGEdge*) const (PGMesh.cpp:196)
==32291==    by 0x8317A2: PGMesh::RemoveNode(PGNode*) (PGMesh.cpp:2084)
==32291==    by 0x83464F: PGMesh::RemoveNodeMergeEdges(PGNode*) (PGMesh.cpp:2722)
==32291==    by 0x8397F1: PGMesh::CleanColinearVerts() (PGMesh.cpp:3852)
==32291==    by 0x9C075D: IntersectTrim(std::vector<TMesh*, std::allocator<TMesh*> >&, std::vector<TMesh*, std::allocator<TMesh*> >&, BndBox&, bool, int, bool, bool, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&, Results*, MeshInfo&) (TMeshUtil.cpp:3380)
==32291==    by 0x7EE727: MeshGeom::IntersectTrim(std::vector<DegenGeom, std::allocator<DegenGeom> >&, bool, int, bool, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&) (MeshGeom.cpp:1311)

I have tried both the .deb package and building from source, both resulted in crashes.

[Rob McDonald]

There are probably two problems.

First, Assertions are not turned on unless you are running a debug build.  CMake defaults to debug builds and you often need to fight it to prevent that.

Does this exact error (or just a crash without an assertion) happen with the Ubuntu package as downloaded from the OpenVSP website, or are you getting the *.deb from somewhere else?

You can also tell that it is a debug build because the file names and line numbers are all listed.

You (or whoever packaged the *.deb) should not be doing a Debug build.  The Debug builds for OpenVSP and VSPAERO are extremely slow.  Your performance will not be what we expect and your user experience will suffer.

In fact, debug builds are so slow that I usually don't use them even for development -- CMake can do what is called a 'Release With Debug Info' build that gives many of the benefits of a Debug build, but without the big performance penalty.

Unfortunately, all this means that debug builds seldom get run and therefore, the asserts that are in the source are seldom exercised.

This can mean that bugs remain hidden (you may have found a real problem here).  Or it can mean that the failure causing the assert is handled gracefully by the calling code and the assert is actually harmless (and should be fixed).

I will try to get a full debug build going and see if I can chase this down.

In the meantime, you should work to force your build to be a Release build and see if that fixes the problem.

If you are getting the *.deb from someplace other than the OpenVSP website, then we need to reach out to the packager to get them to make their build a Release build.

And if you are getting it from the OpenVSP website, then I need to figure out what has gone wrong with my build that it is not a Release build.

Finally -- your subject line says this is the default wing.  Please report exact steps to recreate this issue -- make sure I'm not missing anything.

Rob

[Iordanis Sapidis]

Yes, this is indeed a debug build as I wanted to examine the source of the crash. It also happened with a release build though, just with different errors.

Regarding the .deb file, i downloaded it from the official website https://openvsp.org/download.php (this one is definitely a release build, it does not throw the same assertion error).

Now when I say default wing I mean like the one attached in the zip file, which is essentially just adding a wing without changing anything.

Running this with the official .deb package (OpenVSP 3.50.4 Ubuntu 24.04) the first time I run the VSPAERO solver on that wing I just created and saved, it runs fine. 

The second time I load the same .vsp3 file and try to run the solver, it crashes with various errors (usually segfaults). Deleting the generated files from that first run (*.csf, *.vspgeom etc.) still results in a crash.

Some of the errors, though probably not helpful:

malloc_consolidate(): unaligned fastbin chunk detected
Aborted (core dumped)

double free or corruption (fasttop)
Aborted (core dumped)

munmap_chunk(): invalid pointer
Segmentation fault (core dumped)

[Rob McDonald]

OK, great.  I just wanted to make sure I wasn't accidentally distributing a Debug build...

If the two problems are the same error, then deleting the generated files won't matter -- it is crashing on the OpenVSP side before VSPAERO even has a chance to run.

When you save after running -- and then open VSP again, there should be

WingGeom (Not Shown)

MeshGeom_NGon (Shown, but empty).

The VSPAERO GUI is set up to run on the Shown Set.

So, if you don't Show the WingGeom, it will not run right.  On my machine, it doesn't crash, but it also runs VSPAERO on an empty geometry and is generally unsatisfying.

If you run a second time (after the first successful run), OpenVSP is smart enough to delete the MeshGeom_NGon.  Unfortunately, if you don't delete it manually, it gets saved -- but saving isn't fully implemented for NGon geometry, so it saves the scaffolding with no actual geometry.  Then, when you load it back up, OpenVSP doesn't know that it should delete it before running again.  It should be harmless (it is empty after all), but it is messy.

When you are running the second time, are you literally just opening the file and executing VSPAERO?  Or do you Show the WingGeom before running?  Do you delete the NGon from the Geom Browser?

Rob

[Iordanis Sapidis]

Yes, I make sure that WingGeom is visible and I delete the NGon.

However, the bug appears even if I run and don't save the result.

So for example:

1. Open the project in OpenVSP with just a WingGeom (Shown), run VSPAERO, close without saving.

2. Repeat step 1. Here, it crashes when i try to rerun the solver for some reason.

Sometimes it even crashes on Step 1, so I am not even sure of the exact workflow that causes it, but it always crashes on Step 2.

[Rob McDonald]

Ok, thanks.

I'll need to do some testing on a gcc based machine.  Hopefully it happens there.

If it doesn't, I'll have to try to get closer to your OS setup.  Right now, I don't actually have a dev machine set up that works on deb files.  My linux test boxes are currently on old RHEL/SLES.

Most development happens on MacOS building via CLang - followed by Win / Visual Studio.

Rob

[Iordanis Sapidis]

Thanks for looking into it, I have also attached the gdb backtrace output if it helps.

Seems like a function is being called with a null pointer.

[Rob McDonald]

I think you're running into two problems.

In Debug mode, the assert(0) in PGEdge::ReplaceNode is triggering -- but from all I can tell, it is harmless.  Please comment that out and recompile your debug version.

This is masking your real bug.  With it commented out, you might be able to get the bug to happen.

Next, it will crash when destructing an unsteady or control group in the GUI edit.  This is because by default you're running the Shown Set and the VSPAERO GUI does an update after the chosen set is No-Shown and that causes problems -- that I obviously need to track down.

In the meantime, if you put your Wing into Set_0 and change the Thin Set in VSPAERO to Set_0 -- please try running and re-running to see if you can catch another bug.  This configuration works for me, so if you're seeing something in addition to the above issues, I'd appreciate your help identifying that issue.

Rob

[Iordanis Sapidis]

Ok so I recompiled the debug version with the assert statement commented out.

It now crashed at a different location, I have attached the gdb backtrace in gdb.txt

Also tried your suggestion. It does seem to work with the following configuration (and the wing in Set_0)

However as soon as I change the Thin Set to shown (again with only the wing being visible) it crashes.

I also compiled it with the -fsanitize flag and have attached some logs in fsanitize.txt

Not sure if this is a bug, but the lines 1336 to 1368 in VSP_Geom.C contain this snippet where the last delete statement I think should be 

delete[] UsedTri 

instead of 

delete UsedTri

    int *UsedTri;
    UsedTri = new int[Grid().NumberOfTris() + 1];
    zero_int_array(UsedTri,Grid().NumberOfTris());
    for ( k = 1 ; k <= NumberOfComponents_ ; k++ ) {
       NumberOfThinTris = 0;
       if ( !(ComponentIsThick(k)) ) {
          for ( n = 1 ; n <= Grid().NumberOfTris() ; n++ ) {
             if ( Grid().TriList(n).ComponentID() == ComponentIDForComponent(k) ) UsedTri[n] = 1;
          }
       }
    }
    for ( n = 1 ; n <= Grid().NumberOfTris() ; n++ ) {
       if ( Grid().TriList(n).SurfaceType() != THICK_SURFACE && UsedTri[n] == 0 ) {
          printf("Tri: %d is thin and is not being used... and has componentID: %d \n",n,Grid().TriList(n).ComponentID());fflush(NULL);
       }
    }
    delete UsedTri;

Let me know if this is indeed a bug, I had also opened this issue some days ago and can do a PR if needed.

Thanks!

[Rob McDonald]

I also compiled it with the -fsanitize flag and have attached some logs in fsanitize.txt

Not sure if this is a bug, but the lines 1336 to 1368 in VSP_Geom.C contain this snippet where the last delete statement I think should be 

delete[] UsedTri 

instead of 

delete UsedTri

    int *UsedTri;
    UsedTri = new int[Grid().NumberOfTris() + 1];
    zero_int_array(UsedTri,Grid().NumberOfTris());
    for ( k = 1 ; k <= NumberOfComponents_ ; k++ ) {
       NumberOfThinTris = 0;
       if ( !(ComponentIsThick(k)) ) {
          for ( n = 1 ; n <= Grid().NumberOfTris() ; n++ ) {
             if ( Grid().TriList(n).ComponentID() == ComponentIDForComponent(k) ) UsedTri[n] = 1;
          }
       }
    }
    for ( n = 1 ; n <= Grid().NumberOfTris() ; n++ ) {
       if ( Grid().TriList(n).SurfaceType() != THICK_SURFACE && UsedTri[n] == 0 ) {
          printf("Tri: %d is thin and is not being used... and has componentID: %d \n",n,Grid().TriList(n).ComponentID());fflush(NULL);
       }
    }
    delete UsedTri;

Let me know if this is indeed a bug, I had also opened this issue some days ago and can do a PR if needed.

Yes, this line should read delete[] UsedTri ;

Thanks for that report.

Rob

 

[Rob McDonald]

I've done some more digging.

I've confirmed that simply removing the assert() is the right thing to do.

The other crash (which is what you were originally hitting) I think is actually a race condition because there are two paths that try to Update the m_UnsteadyGroupVec.  When using a 'Shown' Set, OpenVSP would No-Show the geometry when processing it.  Depending on who got there first, it would sometimes work and sometimes crash.

I've modified it so that if you use a Shown Set for either Thin or Thick, OpenVSP will not no-show everything during geometry processing.

This should avoid the Race -- and it will also make it easier for users using the Shown Set to run the analysis repeatedly (since they won't have to re-show everything).

Thanks for these reports.

Rob

[Iordanis Sapidis]

Perfect, is there a branch where you have committed those changes so that I can test them?

[Rob McDonald]

Sure,

https://github.com/ramcdona/OpenVSP/tree/Fixes

Rob

[Iordanis Sapidis]

Yeah this version seems much more stable, thanks.