Skip to content

ISF_Fatras: fix memory leaks in ISF_FatrasToolsG4/G4HadIntProcessor

Liza Mijovic requested to merge lmijovic/athena:master_FATRAS_memleak_t4 into master

The use of new in G4HadIntProcessor causes memory leaks, as reported in ATLASSIM-5687. This MR fixes the leak. The changes only affect simulation with ISF_Fatras.

Details of the changes:

In the original code iFatras::G4HadIntProcessor::initG4RunManager initializes a vector of material properties: std::vector<std::pair<float,std::pair< G4Material*, G4MaterialCutsCouple*> > > m_g4Material; The G4MaterialCutsCouple* is allocated with new with unmatched delete, causing a memory leak.

In the modified code, I take G4MaterialCutsCouple by value: std::vector<std::pair<float,std::pair< G4Material*, G4MaterialCutsCouple> > > m_g4Material; which fixes the leak. I also modify function retrieveG4Material, to avoid making a copy of G4MaterialCutsCouple.

The possible solutions to the leak are constrained by the fact that m_g4Material is passed to external Geant code. This prevents me from solving the leak by eg smart pointers.

Test:

Simulate 3 ttbar events with valgrind before/after the fix. Outputs here: /afs/cern.ch/work/l/lmijovic/public/sim/jira/ATLASSIM-5687_Apr2022/MR_54507

Prior to the fix we have definite leaks corresponding to the G4MaterialCutsCouple* X = new Y allocations:

grep -A 10 'definitely lost' valgrind_prefix.out | grep G4HadIntProcessor.cxx | head -n 7
==11845==    by 0x4329A98F: iFatras::G4HadIntProcessor::initG4RunManager() const (G4HadIntProcessor.cxx:328)
==11845==    by 0x4329B778: iFatras::G4HadIntProcessor::getHadState(ISF::ISFParticle const*, double, Eigen::Matrix<double, 3, 1, 0, 3, 1> const&, Eigen::Matrix<double, 3, 1, 0, 3, 1> const&, Trk::Material const*) const (G4HadIntProcessor.cxx:395)
==11845==    by 0x4329BF83: virtual thunk to iFatras::G4HadIntProcessor::doHadIntOnLayer(ISF::ISFParticle const*, double, Eigen::Matrix<double, 3, 1, 0, 3, 1> const&, Eigen::Matrix<double, 3, 1, 0, 3, 1> const&, Trk::Material const*, Trk::ParticleHypothesis) const (G4HadIntProcessor.cxx:590)
==11845==    by 0x4329AA00: iFatras::G4HadIntProcessor::initG4RunManager() const (G4HadIntProcessor.cxx:335)
==11845==    by 0x4329B778: iFatras::G4HadIntProcessor::getHadState(ISF::ISFParticle const*, double, Eigen::Matrix<double, 3, 1, 0, 3, 1> const&, Eigen::Matrix<double, 3, 1, 0, 3, 1> const&, Trk::Material const*) const (G4HadIntProcessor.cxx:395)
==11845==    by 0x4329BF83: virtual thunk to iFatras::G4HadIntProcessor::doHadIntOnLayer(ISF::ISFParticle const*, double, Eigen::Matrix<double, 3, 1, 0, 3, 1> const&, Eigen::Matrix<double, 3, 1, 0, 3, 1> const&, Trk::Material const*, Trk::ParticleHypothesis) const (G4HadIntProcessor.cxx:590)
==11845==    by 0x4329AA71: iFatras::G4HadIntProcessor::initG4RunManager() const (G4HadIntProcessor.cxx:341)

After the fix the leaks the simulation runs fine:

grep -i 'Athena' valgrind_postfix.out | grep -i 'leaving with code'                                                       
Py:Athena            INFO leaving with code 0: "successful run"

And the memory leaks from G4HadIntProcessor are gone:

grep -A 10 'definitely lost' valgrind_postfix.out | grep G4HadIntProcessor.cxx | head -n 7
-bash-4.2$ 
Edited by Liza Mijovic

Merge request reports