Problem 106

(From C. Green, CDF, FNAL)

Hi,

There are a crop of serious memory-related bugs in objects in the materials
package. Most of these just produce leaks every time a material or element is
reconstructed, but one or two have the potential to either leak memory
cumulatively or corrupt memory.

G4IonisParamElm and G4IonisParamMat both mis-manage the dynamically-allocated
array fShellCorrectionVector: it is not cleanly checked and deallocated on
destruction or reassignment, and the copy constructor erroneously copies the
pointer and assumes ownership when it should be doing a deep copy on freshly
allocated memory.

[As a general comment, one should be *absolutely sure* that members which are
pointers to dynamically allocated memory are initialized in *all* possible
branches of *all* constructors, either to owned memory or 0, if one plans to
delete allocated memory in the destructor. If not explicitly initialized the
pointer may not be 0, in which case the destructor will try to delete unowned
memory and cause corruption.]

In G4Material, the copy and assignment methods are incorrect and do not
correctly deal with owned, dynamically allocated memory. In addition,
fIonisation is not cleaned up in the destructor.

In G4SandiaTable,  neither fMatSandiaMatrix nor
fPhotoAbsorptionCof are initialized. This means that these pointers are
not necessarily zero, and the lines in the destructor:

  if(fMatSandiaMatrix) delete fMatSandiaMatrix ;
  if(fPhotoAbsorptionCof) delete fPhotoAbsorptionCof ;

will lead to unpredictable and irregular crashes due to attempts to
delete unowned memory. In addition, the matrix also contains dynamically
allocated objects by pointer which must be deallocated before the matrix is
destroyed.

Finally with G4SandiaTable, the SandiaIntervals() method is
public, and can therefore be called multiple times outside the control
of the G4SandiaTable class (eg by the physics processes G4PAIonisation
and G4PAIxSection). Here, fPhotoAbsorptionCof is newed without any
attempt to check for and reclaim an existing object, so multiple calls
to SandiaIntervals() will lead to leaks.

In G4Material, under certain circumstances the elements stored by pointer in
theElementVector are dynamically allocated and therefore owned, and in other
circumstances are not. If this is the model that is chosen, one must keep track
of whether theElementVector contains dynamically allocated objects by pointer,
and clean up on destruction or reallocation if and only if required.

You can find CDF's locally altered copies of the relevant files at the following
URLS:

http://purdue-cdf.fnal.gov/CdfCode/source/G4Geometry/<blah>

where <blah> is (eg) source/materials/src/G4Element.cc (see ``File'' field in
this report), with the following caveats:

  The files as presented by your web browser are formatted with line numbers;

  They contain other minor changes such as whitespace formatting or virtual
destructors to allow inheritance.

I hope this report, and the fixes to be found at the URLS specified, are useful.
The problems described here are by no means unique to the materials package or
the files specifically mentioned. It appears to be a fairly common occurrence
that pointers are not initialized in contstructors, or memory not carefully
cleaned up under all circumstances.

Specifically with regard to comparison operators and copy/assignment: much of
the STL assumes that, if copy/assignment and comparison== exist, then a copy of
the object must return a true == comparison. This is not true with many G4
objects which either have incorrect copy/assignment methods, or equality
comparisons which actually check identity rather than equality. Sometimes this
will not matter, depending on the use pattern of the object in question, but
often this could result in a user or some other G4 developer being bitten badly
under certain circumstances.

Thanks for your time,
Chris.