Eliminate a "C++ initialization order fiasco" for geometryregister

Current initialization of the global geometryregister suffers from a
classic 'initialization order fiasco'.  Depending on the order the
compilation units are loaded/linked, the initialization of the global
geometryregisterarray is not guaranteed to happen (and indeed often
does not happen) before it is used.  This leads to entries being
appended before it's initialized (usually 'suceeding, but potentially
causing memory corruption if the segment at that point isn't zeroed),
initialization then happening halfway through (wiping the initial
entries) and then the last entries being the only ones that show up.

The net effect is either a crash at startup, or several geometry types
seeming to be missing.  Eg, step files will oad, but STL files are
just ignored.  The bug is actively observed on, eg, Linux.

This patch implements a simple 'initialize at first access' convention
for the array, eliminating the ordering problem.

I've not reviewed the rest of the source for other potential examples
of the fiasco pattern; this fixes only the geometryregister, since
that was actively biting.
This commit is contained in:
Monty Montgomery 2022-05-22 11:29:10 -04:00
parent e8ec2b3550
commit de7ffc5906
15 changed files with 54 additions and 34 deletions

View File

@ -1673,7 +1673,8 @@ namespace netgen
public:
CSGInit()
{
geometryregister.Append (new CSGeometryRegister);
GeometryRegisterArray& gra = FetchGeometryRegisterArray();
gra.Append (new CSGeometryRegister);
}
};

View File

@ -570,7 +570,8 @@ using namespace netgen;
int Ng_CSG_Init (Tcl_Interp * interp)
{
geometryregister.Append (new CSGeometryVisRegister);
GeometryRegisterArray& gra = FetchGeometryRegisterArray();
gra.Append (new CSGeometryVisRegister);
if (interp == NULL) return TCL_OK;

View File

@ -50,6 +50,7 @@ extern "C" int Ng_geom2d_Init (Tcl_Interp * interp);
int Ng_geom2d_Init (Tcl_Interp * interp)
{
geometryregister.Append (new SplineGeometryVisRegister);
GeometryRegisterArray& gra = FetchGeometryRegisterArray();
gra.Append (new SplineGeometryVisRegister);
return TCL_OK;
}

View File

@ -1100,7 +1100,8 @@ namespace netgen
public:
SplineGeoInit()
{
geometryregister.Append (new SplineGeometryRegister);
GeometryRegisterArray& gra = FetchGeometryRegisterArray();
gra.Append (new SplineGeometryRegister);
}
};

View File

@ -81,9 +81,10 @@ void Ng_LoadGeometry (const char * filename)
return;
}
for (int i = 0; i < geometryregister.Size(); i++)
GeometryRegisterArray &gra = FetchGeometryRegisterArray();
for (int i = 0; i < gra.Size(); i++)
{
NetgenGeometry * hgeom = geometryregister[i]->Load (filename);
NetgenGeometry * hgeom = gra[i]->Load (filename);
if (hgeom)
{
ng_geometry.reset (hgeom);
@ -104,7 +105,8 @@ void Ng_LoadMeshFromStream ( istream & input )
mesh -> Load(input);
SetGlobalMesh (mesh);
ng_geometry = geometryregister.LoadFromMeshFile (input);
GeometryRegisterArray& gra = FetchGeometryRegisterArray();
ng_geometry = gra.LoadFromMeshFile (input);
if (!ng_geometry)
ng_geometry = make_shared<NetgenGeometry>();
@ -259,7 +261,8 @@ void Ng_LoadMesh (const char * filename, ngcore::NgMPI_Comm comm)
shared_ptr<NetgenGeometry> geo;
if(buf.Size()) { // if we had geom-info in the file, take it
istringstream geom_infile(string((const char*)&buf[0], buf.Size()));
geo = geometryregister.LoadFromMeshFile(geom_infile);
GeometryRegisterArray& gra = FetchGeometryRegisterArray();
geo = gra.LoadFromMeshFile(geom_infile);
}
if(geo!=nullptr) {
ng_geometry = geo;

View File

@ -29,9 +29,6 @@ namespace netgen
double GetTolerance() { return tree.GetTolerance(); }
};
DLL_HEADER GeometryRegisterArray geometryregister;
//DLL_HEADER NgArray<GeometryRegister*> geometryregister;
GeometryRegister :: ~GeometryRegister()
{ ; }
@ -1165,4 +1162,10 @@ namespace netgen
}
static RegisterClassForArchive<NetgenGeometry> regnggeo;
GeometryRegisterArray& FetchGeometryRegisterArray (){
static GeometryRegisterArray *geometryregister = new GeometryRegisterArray();
return *geometryregister;
}
}

View File

@ -339,15 +339,12 @@ namespace netgen
public:
virtual ~GeometryRegisterArray()
{
for (int i = 0; i < Size(); i++)
delete (*this)[i];
DeleteAll();
}
virtual shared_ptr<NetgenGeometry> LoadFromMeshFile (istream & ist) const;
};
// extern DLL_HEADER NgArray<GeometryRegister*> geometryregister;
extern DLL_HEADER GeometryRegisterArray geometryregister;
DLL_HEADER GeometryRegisterArray& FetchGeometryRegisterArray ();
}

View File

@ -1660,7 +1660,8 @@ namespace netgen
clusters -> Update();
}
auto geo = geometryregister.LoadFromMeshFile (infile);
GeometryRegisterArray& gra = FetchGeometryRegisterArray();
auto geo = gra.LoadFromMeshFile (infile);
if(geo)
geometry = geo;

View File

@ -838,7 +838,8 @@ DLL_HEADER void ExportNetgenMeshing(py::module &m)
shared_ptr<NetgenGeometry> geo;
if(buf.Size()) { // if we had geom-info in the file, take it
istringstream geom_infile(string((const char*)buf.Data(), buf.Size()));
geo = geometryregister.LoadFromMeshFile(geom_infile);
GeometryRegisterArray& gra = FetchGeometryRegisterArray();
geo = gra.LoadFromMeshFile(geom_infile);
}
if(geo!=nullptr) mesh->SetGeometry(geo);
else if(ng_geometry!=nullptr) mesh->SetGeometry(ng_geometry);

View File

@ -944,7 +944,8 @@ using namespace netgen;
int Ng_occ_Init (Tcl_Interp * interp)
{
geometryregister.Append (new OCCGeometryRegister);
GeometryRegisterArray& gra = FetchGeometryRegisterArray();
gra.Append (new OCCGeometryRegister);
Tcl_CreateCommand (interp, "Ng_SetOCCVisParameters",

View File

@ -3748,7 +3748,8 @@ void STLGeometry :: WriteChartToFile( ChartId chartnumber, filesystem::path file
public:
STLInit()
{
geometryregister.Append (new STLGeometryRegister);
GeometryRegisterArray& gra = FetchGeometryRegisterArray();
gra.Append (new STLGeometryRegister);
}
};

View File

@ -521,8 +521,9 @@ namespace netgen
Tcl_Interp * interp,
int argc, tcl_const char *argv[])
{
for (int i = 0; i < geometryregister.Size(); i++)
geometryregister[i] -> SetParameters (interp);
GeometryRegisterArray& gra = FetchGeometryRegisterArray();
for (int i = 0; i < gra.Size(); i++)
gra[i]->SetParameters (interp);
Ng_SetMeshingParameters (clientData, interp, argc, argv);
@ -564,7 +565,8 @@ using namespace netgen;
extern "C" int Ng_stl_Init (Tcl_Interp * interp);
int Ng_stl_Init (Tcl_Interp * interp)
{
geometryregister.Append (new STLGeometryVisRegister);
GeometryRegisterArray& gra = FetchGeometryRegisterArray();
gra.Append (new STLGeometryVisRegister);
Tcl_CreateCommand (interp, "Ng_SetSTLParameters", Ng_SetSTLParameters,
(ClientData)NULL,

View File

@ -33,8 +33,10 @@ VisualSceneSTLMeshing :: VisualSceneSTLMeshing ()
{
selecttrig = 0;
nodeofseltrig = 1;
stlgeometry->SetSelectTrig(selecttrig);
stlgeometry->SetNodeOfSelTrig(nodeofseltrig);
if(stlgeometry){ // don't let the default initializer crash init
stlgeometry->SetSelectTrig(selecttrig);
stlgeometry->SetNodeOfSelTrig(nodeofseltrig);
}
}
VisualSceneSTLMeshing :: ~VisualSceneSTLMeshing ()

View File

@ -33,8 +33,10 @@ VisualSceneSTLMeshing :: VisualSceneSTLMeshing ()
{
selecttrig = 0;
nodeofseltrig = 1;
stlgeometry->SetSelectTrig(selecttrig);
stlgeometry->SetNodeOfSelTrig(nodeofseltrig);
if(stlgeometry){ // don't let the default initializer crash init
stlgeometry->SetSelectTrig(selecttrig);
stlgeometry->SetNodeOfSelTrig(nodeofseltrig);
}
}
VisualSceneSTLMeshing :: ~VisualSceneSTLMeshing ()

View File

@ -465,15 +465,16 @@ namespace netgen
try
{
for (int i = 0; i < geometryregister.Size(); i++)
GeometryRegisterArray& gra = FetchGeometryRegisterArray();
for (int i = 0; i < gra.Size(); i++)
{
NetgenGeometry * hgeom = geometryregister[i]->Load (lgfilename);
NetgenGeometry * hgeom = gra[i]->Load (lgfilename);
if (hgeom)
{
// delete ng_geometry;
// ng_geometry = hgeom;
ng_geometry = shared_ptr<NetgenGeometry> (hgeom);
geometryregister[i]->SetParameters(interp);
gra[i]->SetParameters(interp);
mesh.reset();
return TCL_OK;
@ -1473,8 +1474,9 @@ namespace netgen
extern void Render(bool blocking);
mparam.render_function = &Render;
for (int i = 0; i < geometryregister.Size(); i++)
geometryregister[i] -> SetParameters (interp);
GeometryRegisterArray& gra = FetchGeometryRegisterArray();
for (int i = 0; i < gra.Size(); i++)
gra[i]->SetParameters (interp);
Ng_SetMeshingParameters (clientData, interp, 0, argv);
@ -1924,9 +1926,10 @@ namespace netgen
{
if (strcmp (vismode, "geometry") == 0)
{
for (int i = 0; i < geometryregister.Size(); i++)
GeometryRegisterArray& gra = FetchGeometryRegisterArray();
for (int i = 0; i < gra.Size(); i++)
{
VisualScene * hvs = geometryregister[i]->GetVisualScene (ng_geometry.get());
VisualScene * hvs = gra[i]->GetVisualScene (ng_geometry.get());
if (hvs)
{
vs = hvs;