Skip to content

WIP: Eliminate a "C++ initialization order fiasco" for geometryregister #125

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

WIP: Eliminate a "C++ initialization order fiasco" for geometryregister #125

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion libsrc/csg/csgeom.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1673,7 +1673,8 @@ namespace netgen
public:
CSGInit()
{
geometryregister.Append (new CSGeometryRegister);
GeometryRegisterArray& gra = FetchGeometryRegisterArray();
gra.Append (new CSGeometryRegister);
}
};

Expand Down
3 changes: 2 additions & 1 deletion libsrc/csg/csgpkg.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;


Expand Down
3 changes: 2 additions & 1 deletion libsrc/geom2d/geom2dpkg.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
3 changes: 2 additions & 1 deletion libsrc/geom2d/geometry2d.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1100,7 +1100,8 @@ namespace netgen
public:
SplineGeoInit()
{
geometryregister.Append (new SplineGeometryRegister);
GeometryRegisterArray& gra = FetchGeometryRegisterArray();
gra.Append (new SplineGeometryRegister);
}
};

Expand Down
11 changes: 7 additions & 4 deletions libsrc/interface/nginterface.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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>();
Expand Down Expand Up @@ -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;
Expand Down
9 changes: 6 additions & 3 deletions libsrc/meshing/basegeom.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,6 @@ namespace netgen
double GetTolerance() { return tree.GetTolerance(); }
};

DLL_HEADER GeometryRegisterArray geometryregister;
//DLL_HEADER NgArray<GeometryRegister*> geometryregister;

GeometryRegister :: ~GeometryRegister()
{ ; }

Expand Down Expand Up @@ -1165,4 +1162,10 @@ namespace netgen
}

static RegisterClassForArchive<NetgenGeometry> regnggeo;

GeometryRegisterArray& FetchGeometryRegisterArray (){
static GeometryRegisterArray *geometryregister = new GeometryRegisterArray();
return *geometryregister;
}

}
7 changes: 2 additions & 5 deletions libsrc/meshing/basegeom.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,15 +339,12 @@ namespace netgen
public:
virtual ~GeometryRegisterArray()
Comment on lines 339 to 340
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid accidental misuse (copying) of the singleton.
Make non-virtual, singleton should never be reimplemented.

Suggested change
public:
virtual ~GeometryRegisterArray()
public:
GeometryRegisterArray(const GeometryRegisterArray &) = delete;
GeometryRegisterArray & operator=(const GeometryRegisterArray &) = delete;
private:
GeometryRegisterArray() = default;
~GeometryRegisterArray() = default;

{
for (int i = 0; i < Size(); i++)
delete (*this)[i];
DeleteAll();
}

virtual shared_ptr<NetgenGeometry> LoadFromMeshFile (istream & ist) const;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LoadFromMeshFile is not reimplemented, but delegated to the registered GeometryRegister::LoadFromMeshFile.

Suggested change
virtual shared_ptr<NetgenGeometry> LoadFromMeshFile (istream & ist) const;
shared_ptr<NetgenGeometry> LoadFromMeshFile (istream & ist) const;

};

// extern DLL_HEADER NgArray<GeometryRegister*> geometryregister;
extern DLL_HEADER GeometryRegisterArray geometryregister;
DLL_HEADER GeometryRegisterArray& FetchGeometryRegisterArray ();
}


Expand Down
3 changes: 2 additions & 1 deletion libsrc/meshing/meshclass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
3 changes: 2 additions & 1 deletion libsrc/meshing/python_mesh.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion libsrc/occ/occpkg.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
3 changes: 2 additions & 1 deletion libsrc/stlgeom/stlgeom.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
};

Expand Down
8 changes: 5 additions & 3 deletions libsrc/stlgeom/stlpkg.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down
6 changes: 4 additions & 2 deletions libsrc/stlgeom/vsstl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Comment on lines +36 to +39
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is unrelated, and incorrect/insufficient (dito below).

stlgeometry is a private member of VisualSceneSTLMeshing, and it must be a nullptr here. (It unfortunately lacks an initializer, and thus may point to arbitrary memory). stlgeometry can only be set with SetGeometry().

See #128

}

VisualSceneSTLMeshing :: ~VisualSceneSTLMeshing ()
Expand Down
6 changes: 4 additions & 2 deletions libsrc/visualization/stlmeshing.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 ()
Expand Down
17 changes: 10 additions & 7 deletions ng/ngpkg.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down