Test bunch-userfile-neutral-particle segfaults

[Jochem Snuverink]

The test bunch-userfile-neutral-particle segfaults. This is due to the fact that in the case of a userfile the particle definition from PrimaryGeneratorAction gets invalidated (by deleting the pointer in BDSBunchUserFile.cc:497, which is pointing to the same location as the one of PrimaryGeneratorAction) and the one from BDSBunch should be used.

I will commit a fix for the test, but possibly the design should be changed so that the invalidated ParticleDefinition can’t be used, or the pointer shouldn’t be deleted (at the cost of a small memory leak, or rather an improved memory cleanup).

[Jochem Snuverink]

fix #272: use event based particle definition for vertex information

→ <<cset c4057dcd6b18>>

[Laurie Nevay]

Thanks for the report. We don’t see a segfault in any of our testing. Which compiler are you using? Also, which operating system?

Yes, there is a divergence in information. BDSPrimaryGeneratorAction is constructed with a BDSParticleDefinition as well as a BDSBunch which contains a BDSParticleDefinition that can be updated dynamically because of the chosen bunch distribution (usually userfile).

Thanks for the fix. I’ll look at the design in the next few days and try to be clearer. We already had a fix for partially stripped ions where the wrong definition was used.

This bit is more complex than I’d like because Geant4 does not allow construction of a particular ion before the primary generator action, which is frustrating.

[Jochem Snuverink]

This was with SL6 gcc6.3 RelWithDebInfo Geant4.10.02.

If you run the test with more particles you’ll likely see a crash at some point since each event memory is read that was already deleted.

I think the design improvement should improve two things:

1. not delete and invalidate the BDSParticleDefinition pointer from BDSPrimaryGeneratorAction (BDSBunch should probably own its own pointer and not a shallow copy).
2. An access function in BDSPrimaryGeneratorAction to the correct BDSParticleDefinition for that event. That will remove some code duplication and be less error-prone.

Also note that I added two comments to the commit c4057dcd6b18

I suspect that in those two lines the wrong BDSParticleDefinition is used, but I am not fully sure.

[Jochem Snuverink]

I will reopen the issue if you are planning to improve the design in any case, then this issue can be used.

[Laurie Nevay]

I've fixed this now. I've redesigned things slightly so that the BDSBunch base class of all bunch distributions owns the particle definition that's used. I've also made better use of the BDSParticleDefinition class throughout and this has simplified several bits of the code.