(one time) memory leaks with GeometryComponent / SimpleComponent building

Issue #157 resolved
Jochem Snuverink created an issue

Coverity discovered the following leak (in 4 places, but always the same pattern), from BDSMagnetOuterFactorPolesBase.cc (and CurviLinearFactory)):

// geometry component
auto endPieceGC = new BDSGeometryComponent(endPieceContainerSolid, endPieceContainerLV);
endPieceGC->RegisterSolid(endPieceCoilSolid);
endPieceGC->RegisterLogicalVolume(endPieceCoilLV);
endPieceGC->RegisterVisAttributes(endPieceCoilVis);
endPieceGC->RegisterSensitiveVolume(endPieceCoilLV);
endPieceGC->SetExtent(BDSExtent(endPieceOuterR, endPieceOuterR, endPieceLength*0.5));
endPieceGC->SetInnerExtent(BDSExtent(endPieceInnerR, endPieceInnerR, endPieceLength*0.5));

endPiece = new BDSSimpleComponent(name + "_end_piece",
                  endPieceGC,
                  endPieceLength);

The pointer to the old discarded instance endPieceGC is leaked. Presumably, one doesn't want to delete it here, since some of the pointers it owns are registered in the new BDSSimpleComponent endPiece.

The whole pattern of first building the GeometryComponent and then copying from it to build SimpleComponent strikes me as slightly odd, especially since the SimpleComponent is already a GeometryComponent.

Why not do something like (with a changed constructor):

endPiece = new BDSSimpleComponent(name + "_end_piece",
                                endPieceContainerSolid, endPieceContainerLV, // removal of the GC pointer, but adding the two container pointers
                                endPieceLength);

// registering
endPiece->RegisterSolid(endPieceCoilSolid);
endPiece->RegisterLogicalVolume(endPieceCoilLV);
endPiece->RegisterVisAttributes(endPieceCoilVis);
endPiece->RegisterSensitiveVolume(endPieceCoilLV);
endPiece->SetExtent(BDSExtent(endPieceOuterR, endPieceOuterR, endPieceLength*0.5));
endPiece->SetInnerExtent(BDSExtent(endPieceInnerR, endPieceInnerR, endPieceLength*0.5));

Then one can get rid of the Inherit method calls in the constructor too.

Comments (1)

  1. Log in to comment